Re: [PATCH next] buffer: guard folio_test_dropbehind() against a NULL b_folio in __bh_submit()

[Ridong Chen <ridong.chen@linux.dev>]

On 9/8/2026 6:07 PM, Jan Kara wrote:
> On Tue 08-09-26 10:36:00, Ridong Chen wrote:
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> Commit 8deae2284976 ("buffer: allow a buffer_head to point at memory
>> outside the page cache") made bh->b_folio optional: jbd2 submits a shadow
>> buffer_head whose data lives in slab (the frozen/escaped copy of a metadata
>> block), and such a buffer carries no folio at all - b_folio stays NULL from
>> alloc_buffer_head() onwards (see the comment in
>> jbd2_journal_write_metadata_buffer()).  That commit audited __bh_submit()
>> and taught bio_add_folio_nofail() and wbc_account_cgroup_owner() to check
>> for a NULL folio, but it missed an earlier dereference:
>>
>> 	if (folio_test_dropbehind(bh->b_folio) && op_is_write(opf))
>> 		bio_set_flag(bio, BIO_COMPLETE_IN_TASK);
>>
>> That test was added by commit a2c924c240e7 ("buffer: set
>> BIO_COMPLETE_IN_TASK for dropbehind writeback"), which predates the
>> NULL-b_folio semantics and so dereferenced b_folio unconditionally.  Once
>> b_folio may be NULL, folio_test_dropbehind() reads NULL->flags and the
>> kernel takes a NULL pointer dereference.
>>
>> This is reached from the jbd2 commit path (bh_submit() -> __bh_submit())
>> whenever a shadow buffer is submitted, i.e. when a metadata block has to be
>> escaped or was copied out, so it is data/timing dependent rather than seen
>> on every commit.  When it does hit, it is deterministic and independent of
>> the dropbehind bit, since the flag test dereferences the folio before
>> examining it.
>>
>> Reproduced on x86_64 with KASAN under filesystem write pressure:
>>
>>    KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
>>    RIP: 0010:__bh_submit+0x21b/0x9d0
>>    Call Trace:
>>     bh_submit+0x15/0x30
>>     jbd2_journal_commit_transaction+0x1c19/0x5ac0
>>     kjournald2+0x1cf/0x760
>>
>> Guard the flag test with a b_folio check, matching the other folio
>> accesses in the same function.
>>
>> Fixes: 8deae2284976 ("buffer: allow a buffer_head to point at memory outside the page cache")
>> Assisted-by: Claude:claude-opus-4-8
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> 
> A similar fix was already posted here [1] and is on its way to upstream.
> 
> 								Honza
> 
> [1] https://lore.kernel.org/all/20260902013357.2815214-1-joseph.qi@linux.alibaba.com/
> 

Apologies, I missed that. Thank you for letting me know.

>> ---
>>   fs/buffer.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/buffer.c b/fs/buffer.c
>> index 427d8a817cd5..f46fa6413032 100644
>> --- a/fs/buffer.c
>> +++ b/fs/buffer.c
>> @@ -1106,7 +1106,8 @@ static void __bh_submit(struct buffer_head *bh, blk_opf_t opf,
>>   
>>   	bio = bio_alloc(bh->b_bdev, 1, opf, GFP_NOIO);
>>   
>> -	if (folio_test_dropbehind(bh->b_folio) && op_is_write(opf))
>> +	if (bh->b_folio && folio_test_dropbehind(bh->b_folio) &&
>> +	    op_is_write(opf))
>>   		bio_set_flag(bio, BIO_COMPLETE_IN_TASK);
>>   
>>   	if (IS_ENABLED(CONFIG_FS_ENCRYPTION))
>> -- 
>> 2.34.1
>>

-- 
Best regards
Ridong