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

I Hsin Cheng
2025-04-25T16:52:48Z

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