Re: [PATCH] gfs2: Fix use-after-free in gfs2_remove_from_journal()

[Andrew Price <anprice@redhat.com>]

On 17/06/2026 10:01, Hongling Zeng wrote:
> The function calls brelse(bh) but then continues to access
> the buffer head through bh->b_private, clear_buffer_dirty(),
> and clear_buffer_uptodate().
> 
> After brelse() decreases the reference count, the buffer head
> may be freed, making the subsequent accesses use-after-free.

When buffers are pinned their refcount is incremented and the brelse() here is only called for pinned buffers so I'm not convinced that there's a bug.

Callers of gfs2_remove_from_journal() also use the bh afterwards so if there was a use-after-free this patch wouldn't fix it.

Did you see a use-after-free in testing?

Andy


> Fix by moving the brelse(bh) call to the end of the function,
> after all accesses to bh have been completed.
> 
> Fixes: e93b100931a4 ("GFS2: Fix slab memory leak in gfs2_bufdata")
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>  fs/gfs2/log.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
> index 78bba8cc10b8..a92c84146de9 100644
> --- a/fs/gfs2/log.c
> +++ b/fs/gfs2/log.c
> @@ -1038,7 +1038,6 @@ void gfs2_remove_from_journal(struct buffer_head *bh, int meta)
>  			set_bit(TR_TOUCHED, &tr->tr_flags);
>  		}
>  		was_pinned = 1;
> -		brelse(bh);
>  	}
>  	if (bd) {
>  		if (bd->bd_tr) {
> @@ -1056,6 +1055,8 @@ void gfs2_remove_from_journal(struct buffer_head *bh, int meta)
>  	}
>  	clear_buffer_dirty(bh);
>  	clear_buffer_uptodate(bh);
> +	if (was_pinned)
> +		brelse(bh);
>  }
>  
>  /**