| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aAu92k-iPbnWBKGz@vaxr-BM6660-BM6360>
Date: Sat, 26 Apr 2025 00:52:42 +0800
From: I Hsin Cheng <richard120310@...il.com>
To: Jan Kara <jack@...e.cz>
Cc: syzbot+de1498ff3a934ac5e8b4@...kaller.appspotmail.com,
viro@...iv.linux.org.uk, brauner@...nel.org,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
jfs-discussion@...ts.sourceforge.net, shaggy@...nel.org,
syzkaller-bugs@...glegroups.com, skhan@...uxfoundation.org,
linux-kernel-mentees@...ts.linux.dev
Subject: Re: [RFC PATCH] fs/buffer: Handle non folio buffer case for
drop_buffer()
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@...kaller.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@...il.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@...e.com>
> SUSE Labs, CR
Powered by blists - more mailing lists