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

[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

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