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@xxxxxxxxxxxxxxxxxxxxxxxxx > 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@xxxxxxxxx> > --- > 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@xxxxxxxx> SUSE Labs, CR
