[RFC PATCH] fs/buffer: Handle non folio buffer case for drop_buffer()

[RFC PATCH] fs/buffer: Handle non folio buffer case for drop_buffer()

[I Hsin Cheng]

When the folio doesn't have any buffers, "folio_buffers(folio)" will
return NULL, causing "buffer_busy(bh)" to dereference a null pointer.
Handle the case and jump to detach the folio if there's no buffer within
it.

Reported-by: syzbot+de1498ff3a934ac5e8b4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=de1498ff3a934ac5e8b4
Fixes: 6439476311a64 ("fs: Convert drop_buffers() to use a folio")
Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
---
syzbot reported a null pointer dereference issue. [1]

If the folio be sent into "drop_buffer()" doesn't have any buffers,
assigning "bh = head" will make "bh" to NULL, and the following
operation of cleaning the buffer will encounter null pointer
dereference.

I checked other use cases of "folio_buffers()", e.g. the one used in
"buffer_check_dirty_writeback()" [2]. They generally use the same
approach to check whether a folio_buffers() return NULL.

I'm not sure whether it's normal for a non-buffer folio to reach inside
"drop_buffers()", if it's not maybe we have to dig more into the problem
and find out where did the buffers of folio get freed or corrupted, let
me know if that's needed and what can I test to help. I'm new to fs
correct me if I'm wrong I'll be happy to learn, and know more about
what's the expected behavior or correct behavior for a folio, thanks !

[1]:
BUG: KASAN: null-ptr-deref in instrument_atomic_read include/linux/instrumented.h:68 [inline]
BUG: KASAN: null-ptr-deref in atomic_read include/linux/atomic/atomic-instrumented.h:32 [inline]
BUG: KASAN: null-ptr-deref in buffer_busy fs/buffer.c:2881 [inline]
BUG: KASAN: null-ptr-deref in drop_buffers+0x6f/0x710 fs/buffer.c:2893
Read of size 4 at addr 0000000000000060 by task kswapd0/74

CPU: 0 UID: 0 PID: 74 Comm: kswapd0 Not tainted 6.12.0-rc1-syzkaller-00031-ge32cde8d2bd7 #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:94 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
 print_report+0xe8/0x550 mm/kasan/report.c:491
 kasan_report+0x143/0x180 mm/kasan/report.c:601
 kasan_check_range+0x282/0x290 mm/kasan/generic.c:189
 instrument_atomic_read include/linux/instrumented.h:68 [inline]
 atomic_read include/linux/atomic/atomic-instrumented.h:32 [inline]
 buffer_busy fs/buffer.c:2881 [inline]
 drop_buffers+0x6f/0x710 fs/buffer.c:2893
 try_to_free_buffers+0x295/0x5f0 fs/buffer.c:2947
 shrink_folio_list+0x240c/0x8cc0 mm/vmscan.c:1432
 evict_folios+0x549b/0x7b50 mm/vmscan.c:4583
 try_to_shrink_lruvec+0x9ab/0xbb0 mm/vmscan.c:4778
 shrink_one+0x3b9/0x850 mm/vmscan.c:4816
 shrink_many mm/vmscan.c:4879 [inline]
 lru_gen_shrink_node mm/vmscan.c:4957 [inline]
 shrink_node+0x3799/0x3de0 mm/vmscan.c:5937
 kswapd_shrink_node mm/vmscan.c:6765 [inline]
 balance_pgdat mm/vmscan.c:6957 [inline]
 kswapd+0x1ca3/0x3700 mm/vmscan.c:7226
 kthread+0x2f0/0x390 kernel/kthread.c:389
 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
 </TASK>

[2]:https://elixir.bootlin.com/linux/v6.14.3/source/fs/buffer.c#L97

Best regards,
I Hsin Cheng
---
 fs/buffer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/buffer.c b/fs/buffer.c
index cc8452f60251..29fd17f78265 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2883,6 +2883,8 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
 	struct buffer_head *head = folio_buffers(folio);
 	struct buffer_head *bh;
 
+	if (!head)
+		goto detach_folio;
 	bh = head;
 	do {
 		if (buffer_busy(bh))
@@ -2897,6 +2899,7 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
 			__remove_assoc_queue(bh);
 		bh = next;
 	} while (bh != head);
+detach_folio:
 	*buffers_to_free = head;
 	folio_detach_private(folio);
 	return true;
-- 
2.43.0

Re: [RFC PATCH] fs/buffer: Handle non folio buffer case for drop_buffer()

[Jan Kara]

On Wed 23-04-25 10:37:03, I Hsin Cheng wrote:
> When the folio doesn't have any buffers, "folio_buffers(folio)" will
> return NULL, causing "buffer_busy(bh)" to dereference a null pointer.
> Handle the case and jump to detach the folio if there's no buffer within
> it.
> 
> Reported-by: syzbot+de1498ff3a934ac5e8b4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=de1498ff3a934ac5e8b4
> Fixes: 6439476311a64 ("fs: Convert drop_buffers() to use a folio")
> Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
> ---
> syzbot reported a null pointer dereference issue. [1]
> 
> If the folio be sent into "drop_buffer()" doesn't have any buffers,
> assigning "bh = head" will make "bh" to NULL, and the following
> operation of cleaning the buffer will encounter null pointer
> dereference.
> 
> I checked other use cases of "folio_buffers()", e.g. the one used in
> "buffer_check_dirty_writeback()" [2]. They generally use the same
> approach to check whether a folio_buffers() return NULL.
> 
> I'm not sure whether it's normal for a non-buffer folio to reach inside
> "drop_buffers()", if it's not maybe we have to dig more into the problem
> and find out where did the buffers of folio get freed or corrupted, let
> me know if that's needed and what can I test to help. I'm new to fs
> correct me if I'm wrong I'll be happy to learn, and know more about
> what's the expected behavior or correct behavior for a folio, thanks !

Thanks for the patch but try_to_free_buffers() is not expected to be called
when there are no buffers. Seeing the stacktrace below, it is unexpected it
got called because filemap_release_folio() calls folio_needs_release()
which should make sure there are indeed buffers attached.

Can you print more about the folio where this happened? In particular it
would be interesting what's in folio->flags, folio->mapping->flags and
folio->mapping->aops (resolved to a symbol). Because either the mapping has
AS_RELEASE_ALWAYS set but then we should have ->releasepage handler, or
have PG_Private bit set without buffers attached to a page but then again
either ->releasepage should be set or there's some bug in fs/buffer.c which
can set PG_Private without attaching buffers (I don't see where that could
be).

								Honza

> 
> [1]:
> BUG: KASAN: null-ptr-deref in instrument_atomic_read include/linux/instrumented.h:68 [inline]
> BUG: KASAN: null-ptr-deref in atomic_read include/linux/atomic/atomic-instrumented.h:32 [inline]
> BUG: KASAN: null-ptr-deref in buffer_busy fs/buffer.c:2881 [inline]
> BUG: KASAN: null-ptr-deref in drop_buffers+0x6f/0x710 fs/buffer.c:2893
> Read of size 4 at addr 0000000000000060 by task kswapd0/74
> 
> CPU: 0 UID: 0 PID: 74 Comm: kswapd0 Not tainted 6.12.0-rc1-syzkaller-00031-ge32cde8d2bd7 #0
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
> Call Trace:
>  <TASK>
>  __dump_stack lib/dump_stack.c:94 [inline]
>  dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
>  print_report+0xe8/0x550 mm/kasan/report.c:491
>  kasan_report+0x143/0x180 mm/kasan/report.c:601
>  kasan_check_range+0x282/0x290 mm/kasan/generic.c:189
>  instrument_atomic_read include/linux/instrumented.h:68 [inline]
>  atomic_read include/linux/atomic/atomic-instrumented.h:32 [inline]
>  buffer_busy fs/buffer.c:2881 [inline]
>  drop_buffers+0x6f/0x710 fs/buffer.c:2893
>  try_to_free_buffers+0x295/0x5f0 fs/buffer.c:2947
>  shrink_folio_list+0x240c/0x8cc0 mm/vmscan.c:1432
>  evict_folios+0x549b/0x7b50 mm/vmscan.c:4583
>  try_to_shrink_lruvec+0x9ab/0xbb0 mm/vmscan.c:4778
>  shrink_one+0x3b9/0x850 mm/vmscan.c:4816
>  shrink_many mm/vmscan.c:4879 [inline]
>  lru_gen_shrink_node mm/vmscan.c:4957 [inline]
>  shrink_node+0x3799/0x3de0 mm/vmscan.c:5937
>  kswapd_shrink_node mm/vmscan.c:6765 [inline]
>  balance_pgdat mm/vmscan.c:6957 [inline]
>  kswapd+0x1ca3/0x3700 mm/vmscan.c:7226
>  kthread+0x2f0/0x390 kernel/kthread.c:389
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
>  </TASK>
> 
> [2]:https://elixir.bootlin.com/linux/v6.14.3/source/fs/buffer.c#L97
> 
> Best regards,
> I Hsin Cheng
> ---
>  fs/buffer.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index cc8452f60251..29fd17f78265 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2883,6 +2883,8 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
>  	struct buffer_head *head = folio_buffers(folio);
>  	struct buffer_head *bh;
>  
> +	if (!head)
> +		goto detach_folio;
>  	bh = head;
>  	do {
>  		if (buffer_busy(bh))
> @@ -2897,6 +2899,7 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
>  			__remove_assoc_queue(bh);
>  		bh = next;
>  	} while (bh != head);
> +detach_folio:
>  	*buffers_to_free = head;
>  	folio_detach_private(folio);
>  	return true;
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [RFC PATCH] fs/buffer: Handle non folio buffer case for drop_buffer()

[I Hsin Cheng]

On Wed, Apr 23, 2025 at 12:13:29PM +0200, Jan Kara wrote:
> On Wed 23-04-25 10:37:03, I Hsin Cheng wrote:
> > When the folio doesn't have any buffers, "folio_buffers(folio)" will
> > return NULL, causing "buffer_busy(bh)" to dereference a null pointer.
> > Handle the case and jump to detach the folio if there's no buffer within
> > it.
> > 
> > Reported-by: syzbot+de1498ff3a934ac5e8b4@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=de1498ff3a934ac5e8b4
> > Fixes: 6439476311a64 ("fs: Convert drop_buffers() to use a folio")
> > Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
> > ---
> > syzbot reported a null pointer dereference issue. [1]
> > 
> > If the folio be sent into "drop_buffer()" doesn't have any buffers,
> > assigning "bh = head" will make "bh" to NULL, and the following
> > operation of cleaning the buffer will encounter null pointer
> > dereference.
> > 
> > I checked other use cases of "folio_buffers()", e.g. the one used in
> > "buffer_check_dirty_writeback()" [2]. They generally use the same
> > approach to check whether a folio_buffers() return NULL.
> > 
> > I'm not sure whether it's normal for a non-buffer folio to reach inside
> > "drop_buffers()", if it's not maybe we have to dig more into the problem
> > and find out where did the buffers of folio get freed or corrupted, let
> > me know if that's needed and what can I test to help. I'm new to fs
> > correct me if I'm wrong I'll be happy to learn, and know more about
> > what's the expected behavior or correct behavior for a folio, thanks !
> 
> Thanks for the patch but try_to_free_buffers() is not expected to be called
> when there are no buffers. Seeing the stacktrace below, it is unexpected it
> got called because filemap_release_folio() calls folio_needs_release()
> which should make sure there are indeed buffers attached.
>

I see, it doesn't make sense to have no buffers inside
try_to_free_buffers() then, I'll dig into it more and send v2.

> Can you print more about the folio where this happened? In particular it
> would be interesting what's in folio->flags, folio->mapping->flags and
> folio->mapping->aops (resolved to a symbol). Because either the mapping has
> AS_RELEASE_ALWAYS set but then we should have ->releasepage handler, or
> have PG_Private bit set without buffers attached to a page but then again
> either ->releasepage should be set or there's some bug in fs/buffer.c which
> can set PG_Private without attaching buffers (I don't see where that could
> be).
> 

Hmm so I suppose when there're buffers attached, the PG_Private bit
should always be set in folio->flags or folio->mapping->flags or
folio->mapping->aops ?

Thanks for your patience and detailed reviewed again, I'll refer back to
you ASAP.

Best regards,
I Hsin Cheng

> 								Honza
> 
> > 
> > [1]:
> > BUG: KASAN: null-ptr-deref in instrument_atomic_read include/linux/instrumented.h:68 [inline]
> > BUG: KASAN: null-ptr-deref in atomic_read include/linux/atomic/atomic-instrumented.h:32 [inline]
> > BUG: KASAN: null-ptr-deref in buffer_busy fs/buffer.c:2881 [inline]
> > BUG: KASAN: null-ptr-deref in drop_buffers+0x6f/0x710 fs/buffer.c:2893
> > Read of size 4 at addr 0000000000000060 by task kswapd0/74
> > 
> > CPU: 0 UID: 0 PID: 74 Comm: kswapd0 Not tainted 6.12.0-rc1-syzkaller-00031-ge32cde8d2bd7 #0
> > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
> > Call Trace:
> >  <TASK>
> >  __dump_stack lib/dump_stack.c:94 [inline]
> >  dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
> >  print_report+0xe8/0x550 mm/kasan/report.c:491
> >  kasan_report+0x143/0x180 mm/kasan/report.c:601
> >  kasan_check_range+0x282/0x290 mm/kasan/generic.c:189
> >  instrument_atomic_read include/linux/instrumented.h:68 [inline]
> >  atomic_read include/linux/atomic/atomic-instrumented.h:32 [inline]
> >  buffer_busy fs/buffer.c:2881 [inline]
> >  drop_buffers+0x6f/0x710 fs/buffer.c:2893
> >  try_to_free_buffers+0x295/0x5f0 fs/buffer.c:2947
> >  shrink_folio_list+0x240c/0x8cc0 mm/vmscan.c:1432
> >  evict_folios+0x549b/0x7b50 mm/vmscan.c:4583
> >  try_to_shrink_lruvec+0x9ab/0xbb0 mm/vmscan.c:4778
> >  shrink_one+0x3b9/0x850 mm/vmscan.c:4816
> >  shrink_many mm/vmscan.c:4879 [inline]
> >  lru_gen_shrink_node mm/vmscan.c:4957 [inline]
> >  shrink_node+0x3799/0x3de0 mm/vmscan.c:5937
> >  kswapd_shrink_node mm/vmscan.c:6765 [inline]
> >  balance_pgdat mm/vmscan.c:6957 [inline]
> >  kswapd+0x1ca3/0x3700 mm/vmscan.c:7226
> >  kthread+0x2f0/0x390 kernel/kthread.c:389
> >  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
> >  ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
> >  </TASK>
> > 
> > [2]:https://elixir.bootlin.com/linux/v6.14.3/source/fs/buffer.c#L97
> > 
> > Best regards,
> > I Hsin Cheng
> > ---
> >  fs/buffer.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/buffer.c b/fs/buffer.c
> > index cc8452f60251..29fd17f78265 100644
> > --- a/fs/buffer.c
> > +++ b/fs/buffer.c
> > @@ -2883,6 +2883,8 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
> >  	struct buffer_head *head = folio_buffers(folio);
> >  	struct buffer_head *bh;
> >  
> > +	if (!head)
> > +		goto detach_folio;
> >  	bh = head;
> >  	do {
> >  		if (buffer_busy(bh))
> > @@ -2897,6 +2899,7 @@ drop_buffers(struct folio *folio, struct buffer_head **buffers_to_free)
> >  			__remove_assoc_queue(bh);
> >  		bh = next;
> >  	} while (bh != head);
> > +detach_folio:
> >  	*buffers_to_free = head;
> >  	folio_detach_private(folio);
> >  	return true;
> > -- 
> > 2.43.0
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

Re: [RFC PATCH] fs/buffer: Handle non folio buffer case for drop_buffer()

[Matthew Wilcox]

On Sat, Apr 26, 2025 at 12:52:42AM +0800, I Hsin Cheng wrote:
> > Can you print more about the folio where this happened? In particular it
> > would be interesting what's in folio->flags, folio->mapping->flags and
> > folio->mapping->aops (resolved to a symbol). Because either the mapping has
> > AS_RELEASE_ALWAYS set but then we should have ->releasepage handler, or
> > have PG_Private bit set without buffers attached to a page but then again
> > either ->releasepage should be set or there's some bug in fs/buffer.c which
> > can set PG_Private without attaching buffers (I don't see where that could
> > be).
> > 
> 
> Hmm so I suppose when there're buffers attached, the PG_Private bit
> should always be set in folio->flags or folio->mapping->flags or
> folio->mapping->aops ?

See folio_attach_private().  I am trying to get rid of PG_private, but
we're not quite there yet.  The other information may help track down
what is going on.