buffer: guard folio_test_dropbehind() against a NULL b_folio in __bh_submit()

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

Posted by Ridong Chen 2 weeks, 3 days ago

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>
---
 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

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

Posted by Jan Kara 2 weeks, 3 days ago

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/

> ---
>  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
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

Posted by Ridong Chen 2 weeks, 3 days ago


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