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

[Hongling Zeng]

在 2026年08月24日 23:35, Andreas Gruenbacher 写道:
> Hi Hongling Zeng,
>
> On Thu, Jun 18, 2026 at 3:58 AM Hongling Zeng <zhongling0719@xxxxxxx> wrote:
> > 在 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'
> a "make C=2 CHECK="smatch" fs/gfs2/" doesn't result in those messages.
> How can I reproduce?
>
> Thanks,
> Andreas
  Hi Andreas and Andy,

  Thank you for the review. The smatch warnings were reported by the Intel
  LKP test robot:

    https://lore.kernel.org/all/202607110908.l7Owhul4-lkp@xxxxxxxxx/

  I will update the patch with a better commit message based on your 
feedback,
  clarifying that this is a defensive improvement to address the pattern
  flagged by static analysis, rather than fixing a real-world 
use-after-free
  bug.

  I'll send a v2 version .

> >     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);
> > > >    }
> > > >
> > > >    /**