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

[Hongling Zeng <zhongling0719@126.com>]

在 2026年08月24日 23:35, Andreas Gruenbacher 写道:
> Hi Hongling Zeng,
>
> On Thu, Jun 18, 2026 at 3:58 AM Hongling Zeng <zhongling0719@126.com> 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@intel.com/

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