KASAN: use-after-free Read in locks_delete_block

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit: 442b8cea2477 Add linux-next specific files for 20181109
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=115dbad5400000
kernel config: https://syzkaller.appspot.com/x/.config?x=2f72bdb11df9fbe8
dashboard link: https://syzkaller.appspot.com/bug?extid=a4a3d526b4157113ec6a
compiler: gcc (GCC) 8.0.1 20180413 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+a4a3d5...@syzkaller.appspotmail.com

device loop0 blocksize: 4096
__find_get_block_slow() failed. block=1, b_blocknr=8
b_state=0x00000029, b_size=512
device loop0 blocksize: 4096
==================================================================
BUG: KASAN: use-after-free in __list_del_entry_valid+0xf1/0x100
lib/list_debug.c:51
Read of size 8 at addr ffff88017eb47b70 by task syz-executor3/13461

CPU: 0 PID: 13461 Comm: syz-executor3 Not tainted 4.20.0-rc1-next-20181109+
#110
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x244/0x39d lib/dump_stack.c:113
print_address_description.cold.7+0x9/0x1ff mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.8+0x242/0x309 mm/kasan/report.c:412
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
__list_del_entry_valid+0xf1/0x100 lib/list_debug.c:51
__list_del_entry include/linux/list.h:117 [inline]
list_del_init include/linux/list.h:159 [inline]
__locks_delete_block fs/locks.c:683 [inline]
locks_delete_block+0xce/0x3d0 fs/locks.c:716
locks_mandatory_area+0x48b/0x6a0 fs/locks.c:1398
rw_verify_area+0x2f2/0x360 fs/read_write.c:386
vfs_write+0x149/0x560 fs/read_write.c:544
ksys_write+0x101/0x260 fs/read_write.c:598
__do_sys_write fs/read_write.c:610 [inline]
__se_sys_write fs/read_write.c:607 [inline]
__x64_sys_write+0x73/0xb0 fs/read_write.c:607
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457569
Code: fd b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 cb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ff2e8194c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000457569
RDX: 0000000000000010 RSI: 0000000020000180 RDI: 0000000000000006
RBP: 000000000072c0e0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007ff2e81956d4
R13: 00000000004c571f R14: 00000000004d9360 R15: 00000000ffffffff

The buggy address belongs to the page:
page:ffffea0005fad1c0 count:0 mapcount:0 mapping:0000000000000000 index:0x0
flags: 0x2fffc0000000000()
raw: 02fffc0000000000 0000000000000000 ffffea0005fad1c8 0000000000000000
raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff88017eb47a00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff88017eb47a80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ffff88017eb47b00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
ffff88017eb47b80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff88017eb47c00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.

[Jeff Layton]

Ouch, crash down in the mandatory locking code. This is with Neil's set
from last week. I haven't merged the series he sent the other day yet,
but they don't seem to be different in this regard.

Looks like the fl_blocked list might have had an entry on it that was
freed without being removed? locks_mandatory_area declares a file_lock
on the stack, but it seems to be initialized properly.

The one weird thing is that locks_mandatory_area sets FL_ACCESS and
FL_SLEEP, but I don't see anything wrong with that right offhand.

Neil, any thoughts?
--
Jeff Layton <jla...@kernel.org>

[syzbot]

syzbot has found a reproducer for the following crash on:

HEAD commit: 442b8cea2477 Add linux-next specific files for 20181109
git tree: linux-next

console output: https://syzkaller.appspot.com/x/log.txt?x=12fbd905400000

kernel config: https://syzkaller.appspot.com/x/.config?x=2f72bdb11df9fbe8
dashboard link: https://syzkaller.appspot.com/bug?extid=a4a3d526b4157113ec6a
compiler: gcc (GCC) 8.0.1 20180413 (experimental)

syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10480d33400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+a4a3d5...@syzkaller.appspotmail.com

==================================================================
BUG: KASAN: use-after-free in __list_del_entry_valid+0xf1/0x100
lib/list_debug.c:51

Read of size 8 at addr ffff8801b27c7c70 by task syz-executor0/6767

CPU: 1 PID: 6767 Comm: syz-executor0 Not tainted 4.20.0-rc1-next-20181109+

#110
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x244/0x39d lib/dump_stack.c:113
print_address_description.cold.7+0x9/0x1ff mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.8+0x242/0x309 mm/kasan/report.c:412
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
__list_del_entry_valid+0xf1/0x100 lib/list_debug.c:51
__list_del_entry include/linux/list.h:117 [inline]
list_del_init include/linux/list.h:159 [inline]
__locks_delete_block fs/locks.c:683 [inline]
locks_delete_block+0xce/0x3d0 fs/locks.c:716
locks_mandatory_area+0x48b/0x6a0 fs/locks.c:1398

locks_verify_truncate include/linux/fs.h:2361 [inline]
do_sys_ftruncate+0x4b2/0x550 fs/open.c:190
__do_sys_ftruncate fs/open.c:204 [inline]
__se_sys_ftruncate fs/open.c:202 [inline]
__x64_sys_ftruncate+0x59/0x80 fs/open.c:202

do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457569
Code: fd b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 cb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00

RSP: 002b:00007f4319e97c78 EFLAGS: 00000246 ORIG_RAX: 000000000000004d
RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 0000000000457569
RDX: 0000000000000000 RSI: 0000000000000039 RDI: 0000000000000004
RBP: 000000000072c040 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f4319e986d4
R13: 00000000004bde51 R14: 00000000004cd048 R15: 00000000ffffffff

The buggy address belongs to the page:

page:ffffea0006c9f1c0 count:0 mapcount:0 mapping:0000000000000000 index:0x0
flags: 0x2fffc0000000000()
raw: 02fffc0000000000 0000000000000000 ffffffff06c90101 0000000000000000

raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:

ffff8801b27c7b00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff8801b27c7b80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ffff8801b27c7c00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
ffff8801b27c7c80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff8801b27c7d00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================

[NeilBrown]

I'm not certain, but probably this:

From: NeilBrown <ne...@suse.com>
Date: Wed, 14 Nov 2018 07:38:05 +1100
Subject: [PATCH] fs/locks: always delete_block after waiting - mandatory locks

The patch
fs/locks: always delete_block after waiting.
should have moved the locks_delete_block() call in
locks_mandatory_area() too.

This might fix the bug:
Reported-by: syzbot+a4a3d5...@syzkaller.appspotmail.com

Signed-off-by: NeilBrown <ne...@suse.com>
---
fs/locks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/locks.c b/fs/locks.c
index f456cd3d9d50..eb0c0b33fb7b 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1436,9 +1436,9 @@ int locks_mandatory_area(struct inode *inode, struct file *filp, loff_t start,
continue;
}

- locks_delete_block(&fl);
break;
}
+ locks_delete_block(&fl);

return error;
}
--
2.14.0.rc0.dirty

[Jeff Layton]

That makes sense. I went ahead and squashed this patch into the earlier
one and pushed the result to my locks-next branch. linux-next should
pick it up soon.

Thanks!
--
Jeff Layton <jla...@kernel.org>

[Dmitry Vyukov]

On Wed, Nov 14, 2018 at 2:36 AM, Jeff Layton <jla...@kernel.org> wrote:
> On Wed, 2018-11-14 at 07:40 +1100, NeilBrown wrote:
>> On Tue, Nov 13 2018, Jeff Layton wrote:
>>
>> > On Mon, 2018-11-12 at 12:34 -0800, syzbot wrote:
>> > > Hello,
>> > >
>> > > syzbot found the following crash on:
>> > >
>> > > HEAD commit: 442b8cea2477 Add linux-next specific files for 20181109
>> > > git tree: linux-next
>> > > console output: https://syzkaller.appspot.com/x/log.txt?x=115dbad5400000
>> > > kernel config: https://syzkaller.appspot.com/x/.config?x=2f72bdb11df9fbe8
>> > > dashboard link: https://syzkaller.appspot.com/bug?extid=a4a3d526b4157113ec6a
>> > > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>> > >
>> > > Unfortunately, I don't have any reproducer for this crash yet.
>> > >
>> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> > > Reported-by: syzbot+a4a3d5...@syzkaller.appspotmail.com

/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\

Hi Neil,

Please include the Reported-by tag next time.

I see the linux-next patch is already update,
so let's tell syzbot that this is fixed here:

#syz fix: fs/locks: always delete_block after waiting.

If the bug is still open on syzbot dashboard:
https://syzkaller.appspot.com#upstream
syzbot will not report new bugs in these functions in future.

Thanks

> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bug...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/b49e02d54460c79c4e5472983f6b9390005881b8.camel%40kernel.org.
> For more options, visit https://groups.google.com/d/optout.

[NeilBrown]

On Thu, Nov 15 2018, Dmitry Vyukov wrote:

> On Wed, Nov 14, 2018 at 2:36 AM, Jeff Layton <jla...@kernel.org> wrote:
>> On Wed, 2018-11-14 at 07:40 +1100, NeilBrown wrote:
>>> On Tue, Nov 13 2018, Jeff Layton wrote:
>>>
>>> > On Mon, 2018-11-12 at 12:34 -0800, syzbot wrote:
>>> > > Hello,
>>> > >
>>> > > syzbot found the following crash on:
>>> > >
>>> > > HEAD commit: 442b8cea2477 Add linux-next specific files for 20181109
>>> > > git tree: linux-next
>>> > > console output: https://syzkaller.appspot.com/x/log.txt?x=115dbad5400000
>>> > > kernel config: https://syzkaller.appspot.com/x/.config?x=2f72bdb11df9fbe8
>>> > > dashboard link: https://syzkaller.appspot.com/bug?extid=a4a3d526b4157113ec6a
>>> > > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>>> > >
>>> > > Unfortunately, I don't have any reproducer for this crash yet.
>>> > >
>>> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>> > > Reported-by: syzbot+a4a3d5...@syzkaller.appspotmail.com
>
> /\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\
>
> Hi Neil,
>
> Please include the Reported-by tag next time.

I did, as you can see below.

When the fix is merged into the patch that introduced the bug, do you
still want the Reported-by there, even though the bug and the fix are no
longer visible? What if I were to completely rewrite the patch - do I
still need the Reported-by??

I'm certainly happy to give credit where due, but keeping a complete
history of past bugs in a single commit seems excessive.
Please help me to understand your needs.

Here you see the Reported-by line that I included.

NeilBrown

[Dmitry Vyukov]

Here is the commit as I see it in linux-next:
https://gist.githubusercontent.com/dvyukov/ac1791c98d95618a48548cef8df84558/raw/a3f819cca2f0bb47db0c2e88d35d020accb069b5/gistfile1.txt
As far as I see it already includes the fix to locks_mandatory_area,
but does not include the tag. Maybe it was merged somehow incorrectly.

This is not so much about credit, but more about proper bug tracking.
But reports on mailing lists are periodically being lost, and then it
also may be hard to understand when a new crash is a new bug which
needs to be reported again or an old lost bug.
syzbot keeps track of all reported bugs and has a notion of
open/active reports that still need human attention:
https://syzkaller.appspot.com/#upstream
and fixed/closed reports that don't need human attention anymore.
The Reported-by tags are intercepted by syzbot and allows it to
understand when a bug is fixed and needs to be closed.
Keeping track of this is important for 2 reasons:
1. Closed/fixed bugs go away from the dashboard, so people don't go
over them again and again.
2. If a bug is closed, syzbot will report new similarly looking bugs
in future (otherwise it will just merge new crashes into the old bug,
because it thinks it's still the old bug happenning).

linux-next is somewhat special because commits are being amended, so a
commit can effectively fix itself. But one way or another syzbot needs
to know about fixes. Reported-by tags take care of all tracking.
Otherwise, a human needs to first notice that there is an already
fixed bug that is still considered open, find the fixing commit and
issue the "#syz fix" command as I did above. This is especially
problematic for linux-next as it changes and bisection does not work.
Here is more info on syzbot bug status tracking:
https://goo.gl/tpsmEJ#bug-status-tracking

> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/87bm6pewnm.fsf%40notabene.neil.brown.name.

[Jeff Layton]

Thanks for the explanation, Dmitry. I've added the tag to the patch in
my tree. It should show up in linux-next soon.

I still find it a little misleading to say that syzbot reported a bug
when it actually found a bug inside an earlier version of the patch, but
I'll just learn to get over it.

Thanks,
--
Jeff Layton <jla...@kernel.org>

[Bruce Fields]

On Sat, Nov 17, 2018 at 08:33:27AM -0500, Jeff Layton wrote:
> Thanks for the explanation, Dmitry. I've added the tag to the patch in
> my tree. It should show up in linux-next soon.
>
> I still find it a little misleading to say that syzbot reported a bug
> when it actually found a bug inside an earlier version of the patch, but
> I'll just learn to get over it.

The usual tag for someone that found a bug in an earlier version of a
patch would be Reviewed-by:. Is there any reason we can't use that
here? The "syzbot+..." email should be enough on its own, I can't see a
reason why their scripts would need to require a particular tag. Or
maybe we could use Tested-by:, or some other tag made up for this case?

I do worry that someone who sees "Reported-by:..." might for example
mistakenly assume that it would help to backport that patch if they see
a similar-looking oops.

--b.

[Dmitry Vyukov]

I see. It may also be picked by scripts that detects patches that need
to be backported to stable because of the "Reported-by: syzbot" tag.
This is somewhat unfortunate.

There is no problem parsing another tag on syzbot side. Does Tested-by
look good to you? If it found a bug in the patch and then it was
fixed, Tested-by looks reasonable. And we also detect
Reported-and-tested-by already because that's what syzbot suggests
after it tested a proposed fix for a bug.

I am somewhat concerned how to spread this information across all
kernel developers. There is effectively no way to do this. We can't
expect people to read docs, they generally don't. I guess I just
document this at "See https://goo.gl/tpsmEJ for more information" and
then we can point other people there if/when this concern pops up
again.

[Jeff Layton]

Tested-by sounds like it might be a reasonable fit. I'll change the
patch in my tree to read that way.

[Dmitry Vyukov]

Turns out this already works (we did not check exact tag, just search
for the right email with a hash). So I added a test for Tested-by tag
and extended the docs:
https://github.com/google/syzkaller/commit/9aca6b5240809308d9078a0a0f0707512c5b0220