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

[Hongling Zeng]

在 2026年06月17日 18:51, Andrew Price 写道:
> 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
 Thanks for your detailed explanation!  this is not a real bug in 
practice, the reference counting protects against real UAF.
 I'm seeing smatch warnings :

      fs/gfs2/log.c:1044 error: dereferencing freed memory 'bh'
      fs/gfs2/log.c:1051 warn: passing freed memory 'bh'

  And there are potential concerns:
  1. Future maintainers might not understand the ref counting semantics
  2. The code pattern (brelse then access) is error-prone
  3. smatch warnings clutter output for real issues
> > 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@xxxxxxxxxx>
> > ---
> >   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);
> >   }
> >   
> >   /**