[Devel] [PATCH rh7 1/3] ext4: add missing brelse() for bh2 in ext4_dx_add_entry()

[Pavel Tikhomirov]

On 4/9/26 15:56, Konstantin Khorenko wrote:
> From: Kemeng Shi <shikemeng at huaweicloud.com>
> 
> Add missing brelse() for bh2 in ext4_dx_add_entry().
> 
> Fixes: ac27a0ec112a ("[PATCH] ext4: initial copy of files from ext3")
> Signed-off-by: Kemeng Shi <shikemeng at huaweicloud.com>
> Reviewed-by: Ojaswin Mujoo <ojaswin at linux.ibm.com>
> Reviewed-by: Zhang Yi <yi.zhang at huawei.com>
> Link: https://patch.msgid.link/20250123162050.2114499-2-shikemeng@huaweicloud.com
> Signed-off-by: Theodore Ts'o <tytso at mit.edu>
> 
> [khorenko: backport to 3.10 - adapted to older ext4_dx_add_entry()
>  code structure.  Applied 3 of 4 upstream hunks:
> 
>  1. ext4_journal_get_write_access(handle, frame->bh) error path:
>     bh2 was just allocated via ext4_append() and not yet stored in
>     any dx_frame, so dx_release() would not free it.  Need explicit
>     brelse(bh2).
> 
>  2. ext4_journal_get_write_access(handle, frames[0].bh) error path
>     inside "if (levels)" block:
>     swap(frame->bh, bh2) has not executed yet, so bh2 is still not
>     referenced by any frame - same situation as (1).
> 
>  3. ext4_handle_dirty_dx_node(handle, dir, bh2) inside "if (levels)"
>     block:
>     Regardless of whether swap(frame->bh, bh2) happened, bh2 holds
>     a buffer that dx_release() does not cover.  Moved brelse(bh2)
>     before the error check since it is needed on both success and
>     error paths.
> 
>  Skipped the 4th upstream hunk (else branch - creating second level
>  index): in our code frame->bh = bh2 is assigned and indirect_levels
>  is set to 1 before the error path, so dx_release() calls
>  brelse(frames[1].bh) which is bh2.  Adding explicit brelse(bh2)
>  there would cause a double-free.]
> 
> https://virtuozzo.atlassian.net/browse/PSBM-161670
> (cherry picked from commit eb640af64db6e5b4b1263d59e4b7849e7fc29352)

Somehow incorrect hash here, should be:

commit eb640af64db6d4702a85ab001b9cc7f4c5dd6abb
Author: Kemeng Shi <shikemeng at huaweicloud.com>
Date:   Fri Jan 24 00:20:48 2025 +0800

    ext4: add missing brelse() for bh2 in ext4_dx_add_entry()

Except that:

Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> ---
>  fs/ext4/namei.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 5279a83f4040..bacdd0630b2c 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -2055,8 +2055,10 @@ static int ext4_dx_add_entry(handle_t *handle, struct dentry *dentry,
>  							   sb->s_blocksize);
>  		BUFFER_TRACE(frame->bh, "get_write_access");
>  		err = ext4_journal_get_write_access(handle, frame->bh);
> -		if (err)
> +		if (err) {
> +			brelse(bh2);
>  			goto journal_error;
> +		}
>  		if (levels) {
>  			unsigned icount1 = icount/2, icount2 = icount - icount1;
>  			unsigned hash2 = dx_get_hash(entries + icount1);
> @@ -2066,8 +2068,10 @@ static int ext4_dx_add_entry(handle_t *handle, struct dentry *dentry,
>  			BUFFER_TRACE(frame->bh, "get_write_access"); /* index root */
>  			err = ext4_journal_get_write_access(handle,
>  							     frames[0].bh);
> -			if (err)
> +			if (err) {
> +				brelse(bh2);
>  				goto journal_error;
> +			}
>  
>  			memcpy((char *) entries2, (char *) (entries + icount1),
>  			       icount2 * sizeof(struct dx_entry));
> @@ -2086,9 +2090,9 @@ static int ext4_dx_add_entry(handle_t *handle, struct dentry *dentry,
>  			dxtrace(dx_show_index("node",
>  			       ((struct dx_node *) bh2->b_data)->entries));
>  			err = ext4_handle_dirty_dx_node(handle, dir, bh2);
> +			brelse(bh2);
>  			if (err)
>  				goto journal_error;
> -			brelse (bh2);
>  		} else {
>  			dxtrace(printk(KERN_DEBUG
>  				       "Creating second level index...\n"));

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.