Re: KASAN: use-after-free Read in corrupted (3)

[syzbot]

syzbot has bisected this bug to:

commit e9db4ef6bf4ca9894bb324c76e01b8f1a16b2650
Author: John Fastabend 
Date:   Sat Jun 30 13:17:47 2018 +

bpf: sockhash fix omitted bucket lock in sock_close

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=11bb4e3da0
start commit:   045df37e Merge branch 'cxgb4-Reference-count-MPS-TCAM-entr..
git tree:   net-next
final crash:https://syzkaller.appspot.com/x/report.txt?x=13bb4e3da0
console output: https://syzkaller.appspot.com/x/log.txt?x=15bb4e3da0
kernel config:  https://syzkaller.appspot.com/x/.config?x=dd16b8dc9d0d210c
dashboard link: https://syzkaller.appspot.com/bug?extid=8a821b383523654227bf
syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=1389f5b5a0

Reported-by: syzbot+8a821b38352365422...@syzkaller.appspotmail.com
Fixes: e9db4ef6bf4c ("bpf: sockhash fix omitted bucket lock in sock_close")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

Re: KASAN: use-after-free Read in corrupted

[Dmitry Vyukov]

On Sun, May 13, 2018 at 12:47 PM, Tetsuo Handa
 wrote:
> Dmitry Vyukov wrote:
>> On Sun, May 13, 2018 at 12:20 PM, Tetsuo Handa
>>  wrote:
>> > Dmitry Vyukov wrote:
>> >> This looks very similar to "KASAN: use-after-free Read in 
>> >> fuse_kill_sb_blk":
>> >> https://groups.google.com/d/msg/syzkaller-bugs/4C4oiBX8vZ0/0NTQRcUYBgAJ
>> >>
>> >> which you fixed with "fuse: don't keep dead fuse_conn at 
>> >> fuse_fill_super().":
>> >> https://groups.google.com/d/msg/syzkaller-bugs/4C4oiBX8vZ0/W6pi8NdbBgAJ
>> >>
>> >> However, here we have use-after-free in fuse_kill_sb_anon instead of
>> >> use_kill_sb_blk. Do you think your patch will fix this as well?
>> >
>> > Yes, for fuse_kill_sb_anon() and fuse_kill_sb_blk() are symmetrical.
>> > I'm waiting for Miklos Szeredi to apply that patch.
>>
>>
>> Thanks for confirming. Let's do:
>>
>> #syz fix: fuse: don't keep dead fuse_conn at fuse_fill_super().
>>
> Excuse me, but that patch is not yet applied to any git tree. Isn't the rule 
> that
>
>   If you forgot to add the Reported-by tag, once the fix for this bug is 
> merged into any tree, please reply to this email with:
>   #syz fix: exact-commit-title

Sorry, the doc is not 100% precisely express the situation. I think
this was discussed several times, but the info is scattered.
What matters in the end is that syzbot discovers the commit using the
title in the tested trees. For example, consider that a commit is
merged into some sub-sub-system tree, its commit title can still
change (for example, an upper subsystem maintainer decided to fix it
up, unlikely, but possible), or the commit can be simply dropped if
upper subsystem maintainer does not like it.
On the other hand, for significant portion of commits once they are
just mailed we are reasonably sure that they will appear upstream
under the original title.

So the full situation is more like:
First you need to decide if you want to deal with this bug sooner or
later (we usually want to deal with them sooner).
If you want to deal with it sooner, the criteria is that you are
"reasonably sure that the commit will reach upstream under this
title", for whatever reason. If it turns out to be wrong, then we will
need to get back to it later and fix up commit title.
If you want to deal with it later, then you can wait till it reaches
upstream and then use syz fix with the actual title.

So I did "syz fix" in the hope that the commit will be taken upstream
as-is, and we don't need to get back to it later.
It also has a useful consequence that the commit that at least was
_meant_ to fix it is recorded on mailing lists. So if it's not fixed
after 3 months, we can find the commit and check if it was forgotten
or renamed or something else.

What do you think of this update to docs:
https://github.com/dvyukov/syzkaller/commit/90075c45aa4422c4656020a3c4e1d6d7a04424ed
Does it make situation clearer?


> ? That's the reason I keep
>
>   KASAN: use-after-free Read in fuse_kill_sb_blk
>   
> https://syzkaller.appspot.com/bug?id=a07a680ed0a9290585ca424546860464dd9658db
>
> report "open()" table but I want keyword column available in the "open()" 
> table
> so that we can announce that "patch is proposed and waiting for review" state.

I wonder if marking the bug with "syz fix" is actually the right way
to handle this. There will be a note about the commit that is supposed
to fix this for both humans reading the email thread and syzbot. Then,
on dashboard it will go to "fix pending" state, which clearly
distinguishes them from other "open" bugs. And we can see that the
commit is still not present in any tested trees by looking at the last
column (Patched: 0/6). I was also thinking of recording time when "syz
fix" command was issued and then marking bugs with red if "syz fix"
was issued more than X months ago, but syzbot still has not discovered
the commit in any trees (useful for detecting typos in commit titles,
or renamed commits).
When we issue such command, we could say something like "Tentatively
marking this as fixed with: ..." to make it clear what happens.
What do you think?

Re: KASAN: use-after-free Read in corrupted

[Tetsuo Handa]

Dmitry Vyukov wrote:
> On Sun, May 13, 2018 at 12:20 PM, Tetsuo Handa
>  wrote:
> > Dmitry Vyukov wrote:
> >> This looks very similar to "KASAN: use-after-free Read in 
> >> fuse_kill_sb_blk":
> >> https://groups.google.com/d/msg/syzkaller-bugs/4C4oiBX8vZ0/0NTQRcUYBgAJ
> >>
> >> which you fixed with "fuse: don't keep dead fuse_conn at 
> >> fuse_fill_super().":
> >> https://groups.google.com/d/msg/syzkaller-bugs/4C4oiBX8vZ0/W6pi8NdbBgAJ
> >>
> >> However, here we have use-after-free in fuse_kill_sb_anon instead of
> >> use_kill_sb_blk. Do you think your patch will fix this as well?
> >
> > Yes, for fuse_kill_sb_anon() and fuse_kill_sb_blk() are symmetrical.
> > I'm waiting for Miklos Szeredi to apply that patch.
> 
> 
> Thanks for confirming. Let's do:
> 
> #syz fix: fuse: don't keep dead fuse_conn at fuse_fill_super().
> 
Excuse me, but that patch is not yet applied to any git tree. Isn't the rule 
that

  If you forgot to add the Reported-by tag, once the fix for this bug is merged 
into any tree, please reply to this email with:
  #syz fix: exact-commit-title 

? That's the reason I keep

  KASAN: use-after-free Read in fuse_kill_sb_blk
  https://syzkaller.appspot.com/bug?id=a07a680ed0a9290585ca424546860464dd9658db

report "open()" table but I want keyword column available in the "open()" table
so that we can announce that "patch is proposed and waiting for review" state.

Re: KASAN: use-after-free Read in corrupted

[Dmitry Vyukov]

On Sun, May 13, 2018 at 12:20 PM, Tetsuo Handa
 wrote:
> Dmitry Vyukov wrote:
>> This looks very similar to "KASAN: use-after-free Read in fuse_kill_sb_blk":
>> https://groups.google.com/d/msg/syzkaller-bugs/4C4oiBX8vZ0/0NTQRcUYBgAJ
>>
>> which you fixed with "fuse: don't keep dead fuse_conn at fuse_fill_super().":
>> https://groups.google.com/d/msg/syzkaller-bugs/4C4oiBX8vZ0/W6pi8NdbBgAJ
>>
>> However, here we have use-after-free in fuse_kill_sb_anon instead of
>> use_kill_sb_blk. Do you think your patch will fix this as well?
>
> Yes, for fuse_kill_sb_anon() and fuse_kill_sb_blk() are symmetrical.
> I'm waiting for Miklos Szeredi to apply that patch.


Thanks for confirming. Let's do:

#syz fix: fuse: don't keep dead fuse_conn at fuse_fill_super().


> static inline struct fuse_conn *get_fuse_conn_super(struct super_block *sb)
> {
> return sb->s_fs_info;
> }
>
> static struct file_system_type fuse_fs_type = {
> .owner  = THIS_MODULE,
> .name   = "fuse",
> .fs_flags   = FS_HAS_SUBTYPE,
> .mount  = fuse_mount,
> .kill_sb= fuse_kill_sb_anon,
> };
>
> static struct file_system_type fuseblk_fs_type = {
> .owner  = THIS_MODULE,
> .name   = "fuseblk",
> .mount  = fuse_mount_blk,
> .kill_sb= fuse_kill_sb_blk,
> .fs_flags   = FS_REQUIRES_DEV | FS_HAS_SUBTYPE,
> };
>
> static void fuse_kill_sb_anon(struct super_block *sb)
> {
> struct fuse_conn *fc = get_fuse_conn_super(sb);
>
> if (fc) {
> down_write(&fc->killsb);
> fc->sb = NULL;
> up_write(&fc->killsb);
> }
>
> kill_anon_super(sb);
> }
>
> static void fuse_kill_sb_blk(struct super_block *sb)
> {
> struct fuse_conn *fc = get_fuse_conn_super(sb);
>
> if (fc) {
> down_write(&fc->killsb);
> fc->sb = NULL;
> up_write(&fc->killsb);
> }
>
> kill_block_super(sb);
> }

Re: KASAN: use-after-free Read in corrupted

[Tetsuo Handa]

Dmitry Vyukov wrote:
> This looks very similar to "KASAN: use-after-free Read in fuse_kill_sb_blk":
> https://groups.google.com/d/msg/syzkaller-bugs/4C4oiBX8vZ0/0NTQRcUYBgAJ
> 
> which you fixed with "fuse: don't keep dead fuse_conn at fuse_fill_super().":
> https://groups.google.com/d/msg/syzkaller-bugs/4C4oiBX8vZ0/W6pi8NdbBgAJ
> 
> However, here we have use-after-free in fuse_kill_sb_anon instead of
> use_kill_sb_blk. Do you think your patch will fix this as well?

Yes, for fuse_kill_sb_anon() and fuse_kill_sb_blk() are symmetrical.
I'm waiting for Miklos Szeredi to apply that patch.

static inline struct fuse_conn *get_fuse_conn_super(struct super_block *sb)
{
return sb->s_fs_info;
}

static struct file_system_type fuse_fs_type = {
.owner  = THIS_MODULE,
.name   = "fuse",
.fs_flags   = FS_HAS_SUBTYPE,
.mount  = fuse_mount,
.kill_sb= fuse_kill_sb_anon,
};

static struct file_system_type fuseblk_fs_type = {
.owner  = THIS_MODULE,
.name   = "fuseblk",
.mount  = fuse_mount_blk,
.kill_sb= fuse_kill_sb_blk,
.fs_flags   = FS_REQUIRES_DEV | FS_HAS_SUBTYPE,
};

static void fuse_kill_sb_anon(struct super_block *sb)
{
struct fuse_conn *fc = get_fuse_conn_super(sb);

if (fc) {
down_write(&fc->killsb);
fc->sb = NULL;
up_write(&fc->killsb);
}

kill_anon_super(sb);
}

static void fuse_kill_sb_blk(struct super_block *sb)
{
struct fuse_conn *fc = get_fuse_conn_super(sb);

if (fc) {
down_write(&fc->killsb);
fc->sb = NULL;
up_write(&fc->killsb);
}

kill_block_super(sb);
}

Re: KASAN: use-after-free Read in corrupted

[Dmitry Vyukov]

On Sun, May 13, 2018 at 10:56 AM, syzbot
 wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:427fbe89261d Merge branch 'next' of git://git.kernel.org/p..
> git tree:   upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=148eb01780
> kernel config:  https://syzkaller.appspot.com/x/.config?x=fcce42b221691ff9
> dashboard link: https://syzkaller.appspot.com/bug?extid=3417712847e7219a60ee
> compiler:   gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=1770c47780
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14ecdbc780
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+3417712847e7219a6...@syzkaller.appspotmail.com

Tetsuo,

This looks very similar to "KASAN: use-after-free Read in fuse_kill_sb_blk":
https://groups.google.com/d/msg/syzkaller-bugs/4C4oiBX8vZ0/0NTQRcUYBgAJ

which you fixed with "fuse: don't keep dead fuse_conn at fuse_fill_super().":
https://groups.google.com/d/msg/syzkaller-bugs/4C4oiBX8vZ0/W6pi8NdbBgAJ

However, here we have use-after-free in fuse_kill_sb_anon instead of
use_kill_sb_blk. Do you think your patch will fix this as well?



> R13:  R14:  R15: 
> CPU: 0 PID: 4564 Comm: syz-executor214 Not tainted 4.17.0-rc4+ #44
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> ==
> Call Trace:
> BUG: KASAN: use-after-free in __lock_acquire+0x3888/0x5140
> kernel/locking/lockdep.c:3310
> Read of size 8 at addr 8801d8d69088 by task syz-executor214/4551
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
>
>  fail_dump lib/fault-inject.c:51 [inline]
>  should_fail.cold.4+0xa/0x1a lib/fault-inject.c:149
>  __should_failslab+0x124/0x180 mm/failslab.c:32
>  should_failslab+0x9/0x14 mm/slab_common.c:1522
>  slab_pre_alloc_hook mm/slab.h:423 [inline]
>  slab_alloc mm/slab.c:3378 [inline]
>  kmem_cache_alloc+0x2af/0x760 mm/slab.c:3552
>  __d_alloc+0xc0/0xd30 fs/dcache.c:1638
>  d_alloc_anon fs/dcache.c:1742 [inline]
>  d_make_root+0x42/0x90 fs/dcache.c:1934
>  fuse_fill_super+0x120e/0x1e20 fs/fuse/inode.c:1131
>  mount_nodev+0x6b/0x110 fs/super.c:1210
>  fuse_mount+0x2c/0x40 fs/fuse/inode.c:1192
>  mount_fs+0xae/0x328 fs/super.c:1267
>  vfs_kern_mount.part.34+0xd4/0x4d0 fs/namespace.c:1037
>  vfs_kern_mount fs/namespace.c:1027 [inline]
>  do_new_mount fs/namespace.c:2518 [inline]
>  do_mount+0x564/0x3070 fs/namespace.c:2848
>  ksys_mount+0x12d/0x140 fs/namespace.c:3064
>  __do_sys_mount fs/namespace.c:3078 [inline]
>  __se_sys_mount fs/namespace.c:3075 [inline]
>  __x64_sys_mount+0xbe/0x150 fs/namespace.c:3075
>  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x447cb9
> RSP: 002b:7f7a75bca918 EFLAGS: 0246 ORIG_RAX: 00a5
> RAX: ffda RBX: 0005 RCX: 00447cb9
> RDX: 004b08d6 RSI: 2340 RDI: 004c7485
> RBP: a001 R08: 7f7a75bca930 R09: 
> R10:  R11: 0246 R12: 
> R13:  R14:  R15: 
> CPU: 1 PID: 4551 Comm: syz-executor214 Not tainted 4.17.0-rc4+ #44
> 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+0x1b9/0x294 lib/dump_stack.c:113
> FAULT_INJECTION: forcing a failure.
> name failslab, interval 1, probability 0, space 0, times 0
>  print_address_description+0x6c/0x20b mm/kasan/report.c:256
>  kasan_report_error mm/kasan/report.c:354 [inline]
>  kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
>  __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
>  __lock_acquire+0x3888/0x5140 kernel/locking/lockdep.c:3310
>  lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3920
>  down_write+0x87/0x120 kernel/locking/rwsem.c:70
>  fuse_kill_sb_anon+0x50/0xb0 fs/fuse/inode.c:1200
>  deactivate_locked_super+0x97/0x100 fs/super.c:316
>  mount_nodev+0xfa/0x110 fs/super.c:1212
>  fuse_mount+0x2c/0x40 fs/fuse/inode.c:1192
>  mount_fs+0xae/0x328 fs/super.c:1267
>  vfs_kern_mount.part.34+0xd4/0x4d0 fs/namespace.c:1037
>  vfs_kern_mount fs/namespace.c:1027 [inline]
>  do_new_mount fs/namespace.c:2518 [inline]
>  do_mount+0x564/0x3070 fs/namespace.c:2848
>  ksys_mount+0x12d/0x140 fs/namespace.c:3064
>  __do_sys_mount fs/namespace.c:3078 [inline]
>  __se_sys_mount fs/namespace.c:3075 [inline]
>  __x64_sys_mount+0xbe/0x150 fs/namespace.c:3075
>  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x447cb9
> RSP: 002b:7f7a75bca918 EFLAGS: 0246 ORIG_RAX: 00a5
> RAX: ffda RBX: 0005 R