Re: [syzbot] [nilfs?] KASAN: slab-out-of-bounds Read in wb_writeback

[Kemeng Shi]

on 4/5/2024 9:23 PM, Jan Kara wrote:
> On Fri 05-04-24 13:05:59, Christian Brauner wrote:
>> On Wed, Apr 03, 2024 at 11:47:17AM +0200, Jan Kara wrote:
>>> On Tue 02-04-24 07:38:25, syzbot wrote:
>>>> syzbot has found a reproducer for the following issue on:
>>>>
>>>> HEAD commit:    c0b832517f62 Add linux-next specific files for 20240402
>>>> git tree:       linux-next
>>>> console+strace: https://syzkaller.appspot.com/x/log.txt?x=14af7dd9180000
>>>> kernel config:  https://syzkaller.appspot.com/x/.config?x=afcaf46d374cec8c
>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=7b219b86935220db6dd8
>>>> compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
>>>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1729f003180000
>>>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17fa4341180000
>>>>
>>>> Downloadable assets:
>>>> disk image: https://storage.googleapis.com/syzbot-assets/0d36ec76edc7/disk-c0b83251.raw.xz
>>>> vmlinux: https://storage.googleapis.com/syzbot-assets/6f9bb4e37dd0/vmlinux-c0b83251.xz
>>>> kernel image: https://storage.googleapis.com/syzbot-assets/2349287b14b7/bzImage-c0b83251.xz
>>>> mounted in repro: https://storage.googleapis.com/syzbot-assets/9760c52a227c/mount_0.gz
>>>>
>>>> IMPORTANT: if you fix the issue, please add the following tag to the commit:
>>>> Reported-by: syzbot+7b219b86935220db6dd8@xxxxxxxxxxxxxxxxxxxxxxxxx
>>>>
>>>> ==================================================================
>>>> BUG: KASAN: slab-out-of-bounds in __lock_acquire+0x78/0x1fd0 kernel/locking/lockdep.c:5005
>>>> Read of size 8 at addr ffff888020485fa8 by task kworker/u8:2/35
>>>
>>> Looks like the writeback cleanups are causing some use-after-free issues.
>>> The code KASAN is complaining about is:
>>>
>>> 		/*
>>> 		 * Nothing written. Wait for some inode to
>>> 		 * become available for writeback. Otherwise
>>> 		 * we'll just busyloop.
>>> 		 */
>>> 		trace_writeback_wait(wb, work);
>>> 		inode = wb_inode(wb->b_more_io.prev);
>>>>>>>> 		spin_lock(&inode->i_lock); <<<<<<
>>> 		spin_unlock(&wb->list_lock);
>>> 		/* This function drops i_lock... */
>>> 		inode_sleep_on_writeback(inode);
>>>
>>> in wb_writeback(). Now looking at the changes indeed the commit
>>> 167d6693deb ("fs/writeback: bail out if there is no more inodes for IO and
>>> queued once") is buggy because it will result in trying to fetch 'inode'
>>> from empty b_more_io list and thus we'll corrupt memory. I think instead of
>>> modifying the condition:
>>>
>>> 		if (list_empty(&wb->b_more_io)) {
>>>
>>> we should do:
>>>
>>> -		if (progress) {
>>> +		if (progress || !queued) {
>>>                         spin_unlock(&wb->list_lock);
>>>                         continue;
>>>                 }
>>>
>>> Kemeng?
>>
>> Fwiw, I observed this on xfstest too the last few days and tracked it
>> down to this series. Here's the splat I got in case it helps:
> 
> OK, since this is apparently causing more issues and Kemeng didn't reply
> yet, here's a fix in the form of the patch. It has passed some basic
> testing. Feel free to fold it into Kemeng's patch so that we don't keep
> linux-next broken longer than necessary. Thanks!
Sorry for the late reply as I was on vacation these days. Also sorry
for the bug introduced. The change looks good to me. Thanks a lot
for helping to fix this in time.

Kemeng
> 
> 								Honza
>