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

在 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 <[email protected]>
>> ---
>>   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);
>>   }
>>   
>>   /**