general protection fault in kernfs_kill_sb (2)

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit: f0ab773f5c96 Merge branch 'akpm' (patches from Andrew)
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=140ce817800000
kernel config: https://syzkaller.appspot.com/x/.config?x=fcce42b221691ff9
dashboard link: https://syzkaller.appspot.com/bug?extid=481ad819c717f6b78df9
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15078e97800000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12bf3697800000

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

RBP: 00007ffc558bda60 R08: 0000000000000000 R09: 0000000300000000
R10: 0000000000000000 R11: 0000000000000246 R12: ffffffffffffffff
R13: 0000000000000004 R14: 0000000000001380 R15: 00007ffc558bd2f8
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 1 PID: 4540 Comm: syz-executor884 Not tainted 4.17.0-rc4+ #43
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:__list_del_entry_valid+0x84/0xf3 lib/list_debug.c:51
RSP: 0018:ffff8801aca6f860 EFLAGS: 00010246
RAX: dffffc0000000000 RBX: ffff8801ac528498 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8801ac5284a0
RBP: ffff8801aca6f878 R08: fffffbfff11cc855 R09: fffffbfff11cc854
R10: ffff8801aca6f878 R11: ffffffff88e642a7 R12: 0000000000000000
R13: 0000000000000000 R14: ffff8801aca6f908 R15: ffff8801ac528498
FS: 0000000000f86880(0000) GS:ffff8801daf00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004b6f3c CR3: 00000001d9730000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
__list_del_entry include/linux/list.h:117 [inline]
list_del include/linux/list.h:125 [inline]
kernfs_kill_sb+0xa0/0x350 fs/kernfs/mount.c:361
sysfs_kill_sb+0x22/0x40 fs/sysfs/mount.c:50
deactivate_locked_super+0x97/0x100 fs/super.c:316
kernfs_mount_ns+0x753/0x8e0 fs/kernfs/mount.c:335
sysfs_mount+0xdf/0x200 fs/sysfs/mount.c:36
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:0x4426a9
RSP: 002b:00007ffc558bd1b8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004426a9
RDX: 00000000200000c0 RSI: 0000000020000080 RDI: 0000000020000040
RBP: 00007ffc558bda60 R08: 0000000000000000 R09: 0000000300000000
R10: 0000000000000000 R11: 0000000000000246 R12: ffffffffffffffff
R13: 0000000000000004 R14: 0000000000001380 R15: 00007ffc558bd2f8
Code: c5 0f 84 cc 00 00 00 48 b8 00 02 00 00 00 00 ad de 49 39 c4 0f 84 a5
00 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 e2 48 c1 ea 03 <80> 3c 02 00
75 5f 49 8b 14 24 48 39 da 0f 85 ba 00 00 00 48 b8
RIP: __list_del_entry_valid+0x84/0xf3 lib/list_debug.c:51 RSP:
ffff8801aca6f860
---[ end trace d148f307a34e229f ]---


---
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.
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
If you want to test a patch for this bug, please reply with:
#syz test: git://repo/address.git branch
and provide the patch inline or as an attachment.
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug
report.
Note: all commands must start from beginning of the line in the email body.

[Tetsuo Handa]

This is what I reported at
https://groups.google.com/d/msg/syzkaller-bugs/ISOJlV2I2QM/qHslGMi3AwAJ .

We are currently waiting for comments from Al Viro.

[Al Viro]

On Sun, May 13, 2018 at 11:19:46AM +0900, Tetsuo Handa wrote:

> This is what I reported at
> https://groups.google.com/d/msg/syzkaller-bugs/ISOJlV2I2QM/qHslGMi3AwAJ .
>
> We are currently waiting for comments from Al Viro.

1) the damn thing is unusable without javashit. Which gets about
the same reaction as sending something.doc in attachment. Please,
find a less obnoxious way to archive the thing (or to generate
URLs that would work without that garbage).

2) deactivate_locked_super() *WILL* be called when fill_super() fails.
Live with it; it allows to simplify a whole lot of cleanup logics
in various filesystems. Again, we are not going for a model where
->kill_sb() is not called for something returned by sget().
Rationale: rarely exercised paths tend to rot, so anything that increases
the duplication of bits and pieces of normal teardown into failure exits
of foo_fill_super() is a bloody bad idea. If anything, we want to take
a lot of stuff out of ->put_super() instances directly into ->kill_sb()
ones, precisely because ->put_super() is only called for fully set up
filesystems.

3) kernfs needs to be fixed. The rest of the dropped commits were
made redundant by 8e04944f0ea8; this one wasn't. Mea culpa.

[Tetsuo Handa]

Al Viro wrote:
> 3) kernfs needs to be fixed. The rest of the dropped commits were
> made redundant by 8e04944f0ea8; this one wasn't. Mea culpa.

Thank you for restoring the kernfs patch (commit e522f0deb929c04e in vfs.git#fixes).

#syz fix: kernfs: deal with kernfs_fill_super() failures



But there remains a refcount bug because deactivate_locked_super() from
kernfs_mount_ns() triggers kobj_ns_drop() from sysfs_kill_sb() via
sb->kill_sb() when kobj_ns_drop() is always called by sysfs_mount()
if kernfs_mount_ns() returned an error.

----------
static void *net_grab_current_ns(void)
{
struct net *ns = current->nsproxy->net_ns;
#ifdef CONFIG_NET_NS
if (ns)
refcount_inc(&ns->passive);
if (ns && !strcmp(current->comm, "a.out"))
printk("net_grab_current_ns: %px %d %d\n", ns,
refcount_read(&ns->passive), refcount_read(&ns->count));
#endif
return ns;
}

void net_drop_ns(void *p)
{
struct net *ns = p;
if (ns && !strcmp(current->comm, "a.out")) {
printk("net_drop_ns: %px %d %d\n", ns,
refcount_read(&ns->passive), refcount_read(&ns->count));
dump_stack();
}
if (ns && refcount_dec_and_test(&ns->passive))
net_free(ns);
}
----------

----------
Normal case
[ 79.283244] net_grab_current_ns: ffff88012e570080 2 1
[ 79.299881] net_drop_ns: ffff88012e570080 2 1
[ 79.303463] CPU: 0 PID: 15294 Comm: a.out Kdump: loaded Tainted: G T 4.17.0-rc3+ #527
[ 79.310509] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017
[ 79.316055] Call Trace:
[ 79.317367] dump_stack+0xe9/0x148
[ 79.319154] net_drop_ns+0xa1/0xb0
[ 79.320903] ? get_net_ns_by_id+0x170/0x170
[ 79.323053] kobj_ns_drop+0x61/0x70
[ 79.324856] sysfs_kill_sb+0x2f/0x40
[ 79.326750] deactivate_locked_super+0x50/0x90
[ 79.329053] deactivate_super+0x61/0x90
[ 79.331032] cleanup_mnt+0x49/0x90
[ 79.332794] __cleanup_mnt+0x16/0x20
[ 79.334641] task_work_run+0xb3/0xf0
[ 79.336485] exit_to_usermode_loop+0x152/0x160
[ 79.338785] do_syscall_64+0x237/0x260
[ 79.340710] entry_SYSCALL_64_after_hwframe+0x49/0xbe

[ 79.357961] net_grab_current_ns: ffff88012e570080 2 1
[ 79.360275] net_drop_ns: ffff88012e570080 2 1
[ 79.362469] CPU: 0 PID: 15294 Comm: a.out Kdump: loaded Tainted: G T 4.17.0-rc3+ #527
[ 79.366436] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017
[ 79.369596] Call Trace:
[ 79.370363] dump_stack+0xe9/0x148
[ 79.371444] net_drop_ns+0xa1/0xb0
[ 79.372504] ? get_net_ns_by_id+0x170/0x170
[ 79.373836] kobj_ns_drop+0x61/0x70
[ 79.374912] sysfs_mount+0xd2/0xf0
[ 79.375976] ? lockdep_init_map+0x9/0x10
[ 79.377343] mount_fs+0x46/0x1a0
[ 79.378365] vfs_kern_mount.part.28+0x67/0x190
[ 79.379850] do_mount+0x7b0/0x11f0
[ 79.381001] ? memdup_user+0x5e/0x90
[ 79.382213] ? copy_mount_options+0x1a4/0x2d0
[ 79.383514] ksys_mount+0xab/0x120
[ 79.384544] __x64_sys_mount+0x26/0x30
[ 79.385761] do_syscall_64+0x7b/0x260
[ 79.386942] entry_SYSCALL_64_after_hwframe+0x49/0xbe
----------

----------
Error case
[ 79.664326] net_grab_current_ns: ffff88012e570080 2 1
[ 79.666073] net_drop_ns: ffff88012e570080 2 1
[ 79.667504] CPU: 1 PID: 15294 Comm: a.out Kdump: loaded Tainted: G T 4.17.0-rc3+ #527
[ 79.670197] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017
[ 79.673282] Call Trace:
[ 79.674041] dump_stack+0xe9/0x148
[ 79.675064] net_drop_ns+0xa1/0xb0
[ 79.676085] ? get_net_ns_by_id+0x170/0x170
[ 79.677326] kobj_ns_drop+0x61/0x70
[ 79.678902] sysfs_kill_sb+0x2f/0x40
[ 79.680144] deactivate_locked_super+0x50/0x90
[ 79.681613] kernfs_mount_ns+0x28f/0x2a0
[ 79.682884] sysfs_mount+0x74/0xf0
[ 79.683906] mount_fs+0x46/0x1a0
[ 79.684879] vfs_kern_mount.part.28+0x67/0x190
[ 79.686193] do_mount+0x7b0/0x11f0
[ 79.687234] ? memdup_user+0x5e/0x90
[ 79.688305] ? copy_mount_options+0x1a4/0x2d0
[ 79.689592] ksys_mount+0xab/0x120
[ 79.690617] __x64_sys_mount+0x26/0x30
[ 79.691735] do_syscall_64+0x7b/0x260
[ 79.692833] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 79.694391] RIP: 0033:0x7fefaf3c4aaa
[ 79.695744] RSP: 002b:00007ffe74af7fd8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
[ 79.698209] RAX: ffffffffffffffda RBX: 000000000000000e RCX: 00007fefaf3c4aaa
[ 79.700386] RDX: 0000000000400896 RSI: 000000000040089c RDI: 0000000000400896
[ 79.702472] RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000002
[ 79.704552] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000400694
[ 79.706624] R13: 00007ffe74af80e0 R14: 0000000000000000 R15: 0000000000000000
[ 79.708802] net_drop_ns: ffff88012e570080 1 1
[ 79.710317] CPU: 1 PID: 15294 Comm: a.out Kdump: loaded Tainted: G T 4.17.0-rc3+ #527
[ 79.713255] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017
[ 79.716704] Call Trace:
[ 79.717463] dump_stack+0xe9/0x148
[ 79.718487] net_drop_ns+0xa1/0xb0
[ 79.719510] ? get_net_ns_by_id+0x170/0x170
[ 79.720771] kobj_ns_drop+0x61/0x70
[ 79.721818] sysfs_mount+0xd2/0xf0
[ 79.722840] mount_fs+0x46/0x1a0
[ 79.723815] vfs_kern_mount.part.28+0x67/0x190
[ 79.725178] do_mount+0x7b0/0x11f0
[ 79.726272] ? memdup_user+0x5e/0x90
[ 79.727361] ? copy_mount_options+0x1a4/0x2d0
[ 79.728811] ksys_mount+0xab/0x120
[ 79.730002] __x64_sys_mount+0x26/0x30
[ 79.731253] do_syscall_64+0x7b/0x260
[ 79.732477] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 79.734039] RIP: 0033:0x7fefaf3c4aaa
[ 79.735136] RSP: 002b:00007ffe74af7fd8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
[ 79.737340] RAX: ffffffffffffffda RBX: 000000000000000e RCX: 00007fefaf3c4aaa
[ 79.739427] RDX: 0000000000400896 RSI: 000000000040089c RDI: 0000000000400896
[ 79.741576] RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000002
[ 79.743771] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000400694
[ 79.746102] R13: 00007ffe74af80e0 R14: 0000000000000000 R15: 0000000000000000
[ 79.748604] ------------[ cut here ]------------
[ 79.750409] ODEBUG: free active (active state 0) object type: timer_list hint: can_stat_update+0x0/0x3b0
[ 79.753308] WARNING: CPU: 1 PID: 15294 at lib/debugobjects.c:329 debug_print_object+0x6a/0x90
[ 79.755786] Modules linked in:
[ 79.756812] CPU: 1 PID: 15294 Comm: a.out Kdump: loaded Tainted: G T 4.17.0-rc3+ #527
[ 79.759725] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017
[ 79.763120] RIP: 0010:debug_print_object+0x6a/0x90
[ 79.764684] RSP: 0018:ffffc900070a3ca0 EFLAGS: 00010086
[ 79.766384] RAX: 0000000000000000 RBX: ffff88012fabea00 RCX: ffffffff8126085e
[ 79.768482] RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff88013a6565b0
[ 79.770537] RBP: ffffc900070a3cb8 R08: 0000000000000000 R09: 0000000000000001
[ 79.772692] R10: ffffc900070a3c18 R11: ffff88012f350140 R12: ffffffff840c6e40
[ 79.774883] R13: ffffffff83ca7711 R14: 0000000000000002 R15: ffff88012e5722c0
[ 79.777012] FS: 00007fefaf897740(0000) GS:ffff88013a640000(0000) knlGS:0000000000000000
[ 79.779762] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 79.779765] CR2: 00007f9542e78090 CR3: 000000012f336002 CR4: 00000000000606e0
[ 79.779814] Call Trace:
[ 79.779825] debug_check_no_obj_freed+0x184/0x1ff
[ 79.779831] kmem_cache_free+0x228/0x280
[ 79.779838] net_drop_ns+0x74/0xb0
[ 79.779845] ? get_net_ns_by_id+0x170/0x170
[ 79.790008] kobj_ns_drop+0x61/0x70
[ 79.791177] sysfs_mount+0xd2/0xf0
[ 79.792311] mount_fs+0x46/0x1a0
[ 79.793343] vfs_kern_mount.part.28+0x67/0x190
[ 79.794653] do_mount+0x7b0/0x11f0
[ 79.795796] ? memdup_user+0x5e/0x90
[ 79.796961] ? copy_mount_options+0x1a4/0x2d0
[ 79.798365] ksys_mount+0xab/0x120
[ 79.799502] __x64_sys_mount+0x26/0x30
[ 79.800780] do_syscall_64+0x7b/0x260
[ 79.801886] entry_SYSCALL_64_after_hwframe+0x49/0xbe
----------

sysfs_mount() seems to be the only user who calls kernfs_mount_ns() with ns != NULL.
I keep waiting for your comment/patch on how to fix this refcount bug.

[Al Viro]

On Mon, May 14, 2018 at 12:20:16PM +0900, Tetsuo Handa wrote:

> But there remains a refcount bug because deactivate_locked_super() from
> kernfs_mount_ns() triggers kobj_ns_drop() from sysfs_kill_sb() via
> sb->kill_sb() when kobj_ns_drop() is always called by sysfs_mount()
> if kernfs_mount_ns() returned an error.

Trivial:

unfuck sysfs_mount()

Signed-off-by: Al Viro <vi...@zeniv.linux.org.uk>
---
diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index b428d317ae92..92682fcc41f6 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -25,7 +25,7 @@ static struct dentry *sysfs_mount(struct file_system_type *fs_type,
{
struct dentry *root;
void *ns;
- bool new_sb;
+ bool new_sb = false;

if (!(flags & SB_KERNMOUNT)) {
if (!kobj_ns_current_may_mount(KOBJ_NS_TYPE_NET))
@@ -35,9 +35,9 @@ static struct dentry *sysfs_mount(struct file_system_type *fs_type,
ns = kobj_ns_grab_current(KOBJ_NS_TYPE_NET);
root = kernfs_mount_ns(fs_type, flags, sysfs_root,
SYSFS_MAGIC, &new_sb, ns);
- if (IS_ERR(root) || !new_sb)
+ if (!new_sb)
kobj_ns_drop(KOBJ_NS_TYPE_NET, ns);
- else if (new_sb)
+ else if (!IS_ERR(root))
root->d_sb->s_iflags |= SB_I_USERNS_VISIBLE;

return root;

[Al Viro]

On Mon, May 14, 2018 at 05:04:15AM +0100, Al Viro wrote:
> diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
> index b428d317ae92..92682fcc41f6 100644
> --- a/fs/sysfs/mount.c
> +++ b/fs/sysfs/mount.c
> @@ -25,7 +25,7 @@ static struct dentry *sysfs_mount(struct file_system_type *fs_type,
> {
> struct dentry *root;
> void *ns;
> - bool new_sb;
> + bool new_sb = false;
>
> if (!(flags & SB_KERNMOUNT)) {
> if (!kobj_ns_current_may_mount(KOBJ_NS_TYPE_NET))
> @@ -35,9 +35,9 @@ static struct dentry *sysfs_mount(struct file_system_type *fs_type,
> ns = kobj_ns_grab_current(KOBJ_NS_TYPE_NET);
> root = kernfs_mount_ns(fs_type, flags, sysfs_root,
> SYSFS_MAGIC, &new_sb, ns);
> - if (IS_ERR(root) || !new_sb)
> + if (!new_sb)
> kobj_ns_drop(KOBJ_NS_TYPE_NET, ns);
> - else if (new_sb)
> + else if (!IS_ERR(root))
> root->d_sb->s_iflags |= SB_I_USERNS_VISIBLE;
>
> return root;

What we want for that kobj_ns_drop() is "no fs instances created" (== no
->kill_sb(), be it now or later, to drop that kobj reference); for setting
->s_iflags - "new instance successfully set up".

That's it; all we need is new_sb that would be accurate on its own.
The problem is with kludging over the cases when it's left uninitialized
(early exits from kernfs_mount_ns()) with IS_ERR(root), which happens to
grab the cases when new_sb *was* set to true. So the fix is to initialize
new_sb properly and get rid of that kludge. Which turns the whole thing
into
if (!new_sb)
...
if (!IS_ERR(root) && new_sb)
...
i.e.
if (!new_sb)
...
else if (!IS_ERR(root))
...

[Stephen Rothwell]

Hi Al,

On Mon, 14 May 2018 05:04:15 +0100 Al Viro <vi...@ZenIV.linux.org.uk> wrote:
>
> On Mon, May 14, 2018 at 12:20:16PM +0900, Tetsuo Handa wrote:
>
> > But there remains a refcount bug because deactivate_locked_super() from
> > kernfs_mount_ns() triggers kobj_ns_drop() from sysfs_kill_sb() via
> > sb->kill_sb() when kobj_ns_drop() is always called by sysfs_mount()
> > if kernfs_mount_ns() returned an error.
>
> Trivial:
>
> unfuck sysfs_mount()
>
> Signed-off-by: Al Viro <vi...@zeniv.linux.org.uk>

I noticed this turn up in linux-next today. Can I ask that you please
add an actual commit message to it before sending it on to Linus.
--
Cheers,
Stephen Rothwell