* [syzbot] [fs?] BUG: unable to handle kernel NULL pointer dereference in path_from_stashed
@ 2024-07-20 16:36 syzbot
2024-07-21 4:50 ` Edward Adam Davis
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: syzbot @ 2024-07-20 16:36 UTC (permalink / raw)
To: brauner, jack, linux-fsdevel, linux-kernel, syzkaller-bugs, viro
Hello,
syzbot found the following issue on:
HEAD commit: 51835949dda3 Merge tag 'net-next-6.11' of git://git.kernel..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1325e60d980000
kernel config: https://syzkaller.appspot.com/x/.config?x=c1c1b0a8065e216
dashboard link: https://syzkaller.appspot.com/bug?extid=34a0ee986f61f15da35d
compiler: arm-linux-gnueabi-gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
userspace arch: arm
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11bd9a5e980000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15726e95980000
Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/8ead8862021c/non_bootable_disk-51835949.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/a129ae4ab997/vmlinux-51835949.xz
kernel image: https://storage.googleapis.com/syzbot-assets/9339fe082652/zImage-51835949.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+34a0ee986f61f15da35d@syzkaller.appspotmail.com
8<--- cut here ---
Unable to handle kernel NULL pointer dereference at virtual address 00000008 when read
[00000008] *pgd=844c0003, *pmd=fe12e003
Internal error: Oops: 205 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 3011 Comm: syz-executor103 Not tainted 6.10.0-syzkaller #0
Hardware name: ARM-Versatile Express
PC is at path_from_stashed+0x1c/0x308 fs/libfs.c:2204
LR is at open_namespace+0x44/0xbc fs/nsfs.c:102
pc : [<8053af54>] lr : [<8054d6f8>] psr: 80000013
sp : df959e80 ip : 84183000 fp : df959ec4
r10: 84183000 r9 : 00000003 r8 : 843f3300
r7 : 82caa250 r6 : 84183000 r5 : 00000000 r4 : 82625878
r3 : df959ecc r2 : 00000008 r1 : 82c95800 r0 : 00000008
Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 30c5387d Table: 843f1a00 DAC: fffffffd
Register r0 information: non-paged memory
Register r1 information: slab kmalloc-1k start 82c95800 pointer offset 0 size 1024
Register r2 information: non-paged memory
Register r3 information: 2-page vmalloc region starting at 0xdf958000 allocated at kernel_clone+0xac/0x3e4 kernel/fork.c:2780
Register r4 information: non-slab/vmalloc memory
Register r5 information: NULL pointer
Register r6 information: slab task_struct start 84183000 pointer offset 0 size 3072
Register r7 information: slab mnt_cache start 82caa240 pointer offset 16 size 184
Register r8 information: slab filp start 843f3300 pointer offset 0 size 160
Register r9 information: non-paged memory
Register r10 information: slab task_struct start 84183000 pointer offset 0 size 3072
Register r11 information: 2-page vmalloc region starting at 0xdf958000 allocated at kernel_clone+0xac/0x3e4 kernel/fork.c:2780
Register r12 information: slab task_struct start 84183000 pointer offset 0 size 3072
Process syz-executor103 (pid: 3011, stack limit = 0xdf958000)
Stack: (0xdf959e80 to 0xdf95a000)
9e80: 841b2400 00000008 df959eb4 df959e98 806e6544 804065ac 00000009 82625878
9ea0: 00000000 84183000 841b2c80 843f3300 00000003 84183000 df959ef4 df959ec8
9ec0: 8054d6f8 8053af44 df959ef4 00000000 00000000 f9244696 82625878 82625878
9ee0: 841b2400 00000008 df959f14 df959ef8 8055271c 8054d6c0 0000ff07 00000000
9f00: 843f3300 00000000 df959fa4 df959f18 8051a7f0 805525a4 000001b2 8020029c
9f20: 84183000 000001b2 df959fac df959f38 8020ba70 8042c724 83f01500 df959f80
9f40: 00000000 843f3300 00000003 82cb0800 df959f7c df959f60 805283b8 8027aebc
9f60: 83f01500 00000003 83f01500 00000003 df959fa4 f9244696 8026b8b0 00000000
9f80: 00000000 0008e058 00000036 8020029c 84183000 00000036 00000000 df959fa8
9fa0: 80200060 8051a6c8 00000000 00000000 00000003 0000ff07 00000000 00000000
9fc0: 00000000 00000000 0008e058 00000036 7ee54e0c 00000000 00000001 00000000
9fe0: 7ee54c70 7ee54c60 0001064c 0002e7a0 00000010 00000003 00000000 00000000
Call trace:
[<8053af38>] (path_from_stashed) from [<8054d6f8>] (open_namespace+0x44/0xbc fs/nsfs.c:102)
r10:84183000 r9:00000003 r8:843f3300 r7:841b2c80 r6:84183000 r5:00000000
r4:82625878
[<8054d6b4>] (open_namespace) from [<8055271c>] (pidfd_ioctl+0x184/0x4c4 fs/pidfs.c:196)
r6:00000008 r5:841b2400 r4:82625878
[<80552598>] (pidfd_ioctl) from [<8051a7f0>] (vfs_ioctl fs/ioctl.c:51 [inline])
[<80552598>] (pidfd_ioctl) from [<8051a7f0>] (do_vfs_ioctl fs/ioctl.c:861 [inline])
[<80552598>] (pidfd_ioctl) from [<8051a7f0>] (__do_sys_ioctl fs/ioctl.c:905 [inline])
[<80552598>] (pidfd_ioctl) from [<8051a7f0>] (sys_ioctl+0x134/0xda4 fs/ioctl.c:893)
r7:00000000 r6:843f3300 r5:00000000 r4:0000ff07
[<8051a6bc>] (sys_ioctl) from [<80200060>] (ret_fast_syscall+0x0/0x1c arch/arm/mm/proc-v7.S:67)
Exception stack(0xdf959fa8 to 0xdf959ff0)
9fa0: 00000000 00000000 00000003 0000ff07 00000000 00000000
9fc0: 00000000 00000000 0008e058 00000036 7ee54e0c 00000000 00000001 00000000
9fe0: 7ee54c70 7ee54c60 0001064c 0002e7a0
r10:00000036 r9:84183000 r8:8020029c r7:00000036 r6:0008e058 r5:00000000
r4:00000000
Code: e24dd01c e1a07001 e5911004 ee1dcf70 (e5905000)
---[ end trace 0000000000000000 ]---
----------------
Code disassembly (best guess):
0: e24dd01c sub sp, sp, #28
4: e1a07001 mov r7, r1
8: e5911004 ldr r1, [r1, #4]
c: ee1dcf70 mrc 15, 0, ip, cr13, cr0, {3}
* 10: e5905000 ldr r5, [r0] <-- trapping instruction
---
This report 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 syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [syzbot] [fs?] BUG: unable to handle kernel NULL pointer dereference in path_from_stashed 2024-07-20 16:36 [syzbot] [fs?] BUG: unable to handle kernel NULL pointer dereference in path_from_stashed syzbot @ 2024-07-21 4:50 ` Edward Adam Davis 2024-07-21 4:57 ` syzbot 2024-07-21 5:14 ` Edward Adam Davis 2024-07-21 6:23 ` [PATCH] fs/pidfs: when time ns disabled add check for ioctl Edward Adam Davis 2 siblings, 1 reply; 10+ messages in thread From: Edward Adam Davis @ 2024-07-21 4:50 UTC (permalink / raw) To: syzbot+34a0ee986f61f15da35d; +Cc: linux-kernel, syzkaller-bugs time_ns is null ? #syz test: upstream 5e0497553643 diff --git a/fs/pidfs.c b/fs/pidfs.c index c9cb14181def..fdae58eb1d98 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -168,6 +168,8 @@ static long pidfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg) case PIDFD_GET_TIME_NAMESPACE: get_time_ns(nsp->time_ns); ns_common = to_ns_common(nsp->time_ns); + if (!nsp->time_ns) + return -EINVAL; break; case PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE: get_time_ns(nsp->time_ns_for_children); ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [syzbot] [fs?] BUG: unable to handle kernel NULL pointer dereference in path_from_stashed 2024-07-21 4:50 ` Edward Adam Davis @ 2024-07-21 4:57 ` syzbot 0 siblings, 0 replies; 10+ messages in thread From: syzbot @ 2024-07-21 4:57 UTC (permalink / raw) To: eadavis, linux-kernel, syzkaller-bugs Hello, syzbot tried to test the proposed patch but the build/boot failed: failed to apply patch: checking file fs/pidfs.c Hunk #1 FAILED at 168. 1 out of 1 hunk FAILED Tested on: commit: 5e049755 Merge branch 'link_path_walk' git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git kernel config: https://syzkaller.appspot.com/x/.config?x=c1c1b0a8065e216 dashboard link: https://syzkaller.appspot.com/bug?extid=34a0ee986f61f15da35d compiler: userspace arch: arm patch: https://syzkaller.appspot.com/x/patch.diff?x=13ea160d980000 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [syzbot] [fs?] BUG: unable to handle kernel NULL pointer dereference in path_from_stashed 2024-07-20 16:36 [syzbot] [fs?] BUG: unable to handle kernel NULL pointer dereference in path_from_stashed syzbot 2024-07-21 4:50 ` Edward Adam Davis @ 2024-07-21 5:14 ` Edward Adam Davis 2024-07-21 5:41 ` syzbot 2024-07-21 6:23 ` [PATCH] fs/pidfs: when time ns disabled add check for ioctl Edward Adam Davis 2 siblings, 1 reply; 10+ messages in thread From: Edward Adam Davis @ 2024-07-21 5:14 UTC (permalink / raw) To: syzbot+34a0ee986f61f15da35d; +Cc: linux-kernel, syzkaller-bugs time_ns is null ? #syz test: upstream 51835949dda3 diff --git a/fs/pidfs.c b/fs/pidfs.c index c9cb14181def..fdae58eb1d98 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -168,6 +168,8 @@ static long pidfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg) case PIDFD_GET_TIME_NAMESPACE: get_time_ns(nsp->time_ns); ns_common = to_ns_common(nsp->time_ns); + if (!nsp->time_ns) + return -EINVAL; break; case PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE: get_time_ns(nsp->time_ns_for_children); ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [syzbot] [fs?] BUG: unable to handle kernel NULL pointer dereference in path_from_stashed 2024-07-21 5:14 ` Edward Adam Davis @ 2024-07-21 5:41 ` syzbot 0 siblings, 0 replies; 10+ messages in thread From: syzbot @ 2024-07-21 5:41 UTC (permalink / raw) To: eadavis, linux-kernel, syzkaller-bugs Hello, syzbot has tested the proposed patch and the reproducer did not trigger any issue: Reported-by: syzbot+34a0ee986f61f15da35d@syzkaller.appspotmail.com Tested-by: syzbot+34a0ee986f61f15da35d@syzkaller.appspotmail.com Tested on: commit: 51835949 Merge tag 'net-next-6.11' of git://git.kernel.. git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git console output: https://syzkaller.appspot.com/x/log.txt?x=1421b72d980000 kernel config: https://syzkaller.appspot.com/x/.config?x=c1c1b0a8065e216 dashboard link: https://syzkaller.appspot.com/bug?extid=34a0ee986f61f15da35d compiler: arm-linux-gnueabi-gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40 userspace arch: arm patch: https://syzkaller.appspot.com/x/patch.diff?x=100ea8b1980000 Note: testing is done by a robot and is best-effort only. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] fs/pidfs: when time ns disabled add check for ioctl 2024-07-20 16:36 [syzbot] [fs?] BUG: unable to handle kernel NULL pointer dereference in path_from_stashed syzbot 2024-07-21 4:50 ` Edward Adam Davis 2024-07-21 5:14 ` Edward Adam Davis @ 2024-07-21 6:23 ` Edward Adam Davis 2024-07-21 10:59 ` Matthew Wilcox ` (3 more replies) 2 siblings, 4 replies; 10+ messages in thread From: Edward Adam Davis @ 2024-07-21 6:23 UTC (permalink / raw) To: syzbot+34a0ee986f61f15da35d Cc: brauner, jack, linux-fsdevel, linux-kernel, syzkaller-bugs, viro syzbot call pidfd_ioctl() with cmd "PIDFD_GET_TIME_NAMESPACE" and disabled CONFIG_TIME_NS, since time_ns is NULL, it will make NULL ponter deref in open_namespace. Reported-and-tested-by: syzbot+34a0ee986f61f15da35d@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=34a0ee986f61f15da35d Signed-off-by: Edward Adam Davis <eadavis@qq.com> --- fs/pidfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/pidfs.c b/fs/pidfs.c index c9cb14181def..fe0ddab48f57 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -168,6 +168,8 @@ static long pidfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg) case PIDFD_GET_TIME_NAMESPACE: get_time_ns(nsp->time_ns); ns_common = to_ns_common(nsp->time_ns); + if (!nsp->time_ns) + return -EINVAL; break; case PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE: get_time_ns(nsp->time_ns_for_children); -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] fs/pidfs: when time ns disabled add check for ioctl 2024-07-21 6:23 ` [PATCH] fs/pidfs: when time ns disabled add check for ioctl Edward Adam Davis @ 2024-07-21 10:59 ` Matthew Wilcox 2024-07-22 8:00 ` Christian Brauner ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Matthew Wilcox @ 2024-07-21 10:59 UTC (permalink / raw) To: Edward Adam Davis Cc: syzbot+34a0ee986f61f15da35d, brauner, jack, linux-fsdevel, linux-kernel, syzkaller-bugs, viro On Sun, Jul 21, 2024 at 02:23:12PM +0800, Edward Adam Davis wrote: > syzbot call pidfd_ioctl() with cmd "PIDFD_GET_TIME_NAMESPACE" and disabled > CONFIG_TIME_NS, since time_ns is NULL, it will make NULL ponter deref in > open_namespace. what about PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fs/pidfs: when time ns disabled add check for ioctl 2024-07-21 6:23 ` [PATCH] fs/pidfs: when time ns disabled add check for ioctl Edward Adam Davis 2024-07-21 10:59 ` Matthew Wilcox @ 2024-07-22 8:00 ` Christian Brauner 2024-07-22 13:13 ` [PATCH 1/2] pidfs: handle kernels without namespaces cleanly Christian Brauner 2024-07-22 13:13 ` [PATCH 2/2] pidfs: add selftests for new namespace ioctls Christian Brauner 3 siblings, 0 replies; 10+ messages in thread From: Christian Brauner @ 2024-07-22 8:00 UTC (permalink / raw) To: Edward Adam Davis Cc: syzbot+34a0ee986f61f15da35d, jack, linux-fsdevel, linux-kernel, syzkaller-bugs, viro On Sun, Jul 21, 2024 at 02:23:12PM GMT, Edward Adam Davis wrote: > syzbot call pidfd_ioctl() with cmd "PIDFD_GET_TIME_NAMESPACE" and disabled > CONFIG_TIME_NS, since time_ns is NULL, it will make NULL ponter deref in > open_namespace. > > Reported-and-tested-by: syzbot+34a0ee986f61f15da35d@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=34a0ee986f61f15da35d > Signed-off-by: Edward Adam Davis <eadavis@qq.com> > --- Hm, nsproxy is really messy in that regard. Some namespaces will always be set to init_<type>_ns and others will be set to NULL. That's an invitation for bugs such as this. Imho the correct fix is to change nsproxy to always set nsp-><type>_ns to init_<type>_ns and no code ever needs to worry about dereferencing NULL. But that'll require more changes so this seems an appropriate fix for now. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] pidfs: handle kernels without namespaces cleanly 2024-07-21 6:23 ` [PATCH] fs/pidfs: when time ns disabled add check for ioctl Edward Adam Davis 2024-07-21 10:59 ` Matthew Wilcox 2024-07-22 8:00 ` Christian Brauner @ 2024-07-22 13:13 ` Christian Brauner 2024-07-22 13:13 ` [PATCH 2/2] pidfs: add selftests for new namespace ioctls Christian Brauner 3 siblings, 0 replies; 10+ messages in thread From: Christian Brauner @ 2024-07-22 13:13 UTC (permalink / raw) To: linux-fsdevel Cc: Christian Brauner, Edward Adam Davis, syzbot+34a0ee986f61f15da35d, jack, linux-kernel, syzkaller-bugs, viro The nsproxy structure contains nearly all of the namespaces associated with a task. When a given namespace type is not supported by this kernel the rules whether the corresponding pointer in struct nsproxy is NULL or always init_<ns_type>_ns differ per namespace. Ideally, that wouldn't be the case and for all namespace types we'd always set it to init_<ns_type>_ns when the corresponding namespace type isn't supported. Make sure we handle all namespaces where the pointer in struct nsproxy can be NULL when the namespace type isn't supported. Fixes: 5b08bd408534 ("pidfs: allow retrieval of namespace file descriptors") # mainline only Signed-off-by: Christian Brauner <brauner@kernel.org> --- fs/pidfs.c | 65 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/fs/pidfs.c b/fs/pidfs.c index fe0ddab48f57..7ffdc88dfb52 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -119,7 +119,7 @@ static long pidfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg) struct task_struct *task __free(put_task) = NULL; struct nsproxy *nsp __free(put_nsproxy) = NULL; struct pid *pid = pidfd_pid(file); - struct ns_common *ns_common; + struct ns_common *ns_common = NULL; if (arg) return -EINVAL; @@ -146,54 +146,73 @@ static long pidfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg) switch (cmd) { /* Namespaces that hang of nsproxy. */ case PIDFD_GET_CGROUP_NAMESPACE: - get_cgroup_ns(nsp->cgroup_ns); - ns_common = to_ns_common(nsp->cgroup_ns); + if (IS_ENABLED(CONFIG_CGROUPS)) { + get_cgroup_ns(nsp->cgroup_ns); + ns_common = to_ns_common(nsp->cgroup_ns); + } break; case PIDFD_GET_IPC_NAMESPACE: - get_ipc_ns(nsp->ipc_ns); - ns_common = to_ns_common(nsp->ipc_ns); + if (IS_ENABLED(CONFIG_IPC_NS)) { + get_ipc_ns(nsp->ipc_ns); + ns_common = to_ns_common(nsp->ipc_ns); + } break; case PIDFD_GET_MNT_NAMESPACE: get_mnt_ns(nsp->mnt_ns); ns_common = to_ns_common(nsp->mnt_ns); break; case PIDFD_GET_NET_NAMESPACE: - ns_common = to_ns_common(nsp->net_ns); - get_net_ns(ns_common); + if (IS_ENABLED(CONFIG_NET_NS)) { + ns_common = to_ns_common(nsp->net_ns); + get_net_ns(ns_common); + } break; case PIDFD_GET_PID_FOR_CHILDREN_NAMESPACE: - get_pid_ns(nsp->pid_ns_for_children); - ns_common = to_ns_common(nsp->pid_ns_for_children); + if (IS_ENABLED(CONFIG_PID_NS)) { + get_pid_ns(nsp->pid_ns_for_children); + ns_common = to_ns_common(nsp->pid_ns_for_children); + } break; case PIDFD_GET_TIME_NAMESPACE: - get_time_ns(nsp->time_ns); - ns_common = to_ns_common(nsp->time_ns); - if (!nsp->time_ns) - return -EINVAL; + if (IS_ENABLED(CONFIG_TIME_NS)) { + get_time_ns(nsp->time_ns); + ns_common = to_ns_common(nsp->time_ns); + } break; case PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE: - get_time_ns(nsp->time_ns_for_children); - ns_common = to_ns_common(nsp->time_ns_for_children); + if (IS_ENABLED(CONFIG_TIME_NS)) { + get_time_ns(nsp->time_ns_for_children); + ns_common = to_ns_common(nsp->time_ns_for_children); + } break; case PIDFD_GET_UTS_NAMESPACE: - get_uts_ns(nsp->uts_ns); - ns_common = to_ns_common(nsp->uts_ns); + if (IS_ENABLED(CONFIG_UTS_NS)) { + get_uts_ns(nsp->uts_ns); + ns_common = to_ns_common(nsp->uts_ns); + } break; /* Namespaces that don't hang of nsproxy. */ case PIDFD_GET_USER_NAMESPACE: - rcu_read_lock(); - ns_common = to_ns_common(get_user_ns(task_cred_xxx(task, user_ns))); - rcu_read_unlock(); + if (IS_ENABLED(CONFIG_USER_NS)) { + rcu_read_lock(); + ns_common = to_ns_common(get_user_ns(task_cred_xxx(task, user_ns))); + rcu_read_unlock(); + } break; case PIDFD_GET_PID_NAMESPACE: - rcu_read_lock(); - ns_common = to_ns_common(get_pid_ns(task_active_pid_ns(task))); - rcu_read_unlock(); + if (IS_ENABLED(CONFIG_PID_NS)) { + rcu_read_lock(); + ns_common = to_ns_common( get_pid_ns(task_active_pid_ns(task))); + rcu_read_unlock(); + } break; default: return -ENOIOCTLCMD; } + if (!ns_common) + return -EOPNOTSUPP; + /* open_namespace() unconditionally consumes the reference */ return open_namespace(ns_common); } -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] pidfs: add selftests for new namespace ioctls 2024-07-21 6:23 ` [PATCH] fs/pidfs: when time ns disabled add check for ioctl Edward Adam Davis ` (2 preceding siblings ...) 2024-07-22 13:13 ` [PATCH 1/2] pidfs: handle kernels without namespaces cleanly Christian Brauner @ 2024-07-22 13:13 ` Christian Brauner 3 siblings, 0 replies; 10+ messages in thread From: Christian Brauner @ 2024-07-22 13:13 UTC (permalink / raw) To: linux-fsdevel Cc: Christian Brauner, Edward Adam Davis, syzbot+34a0ee986f61f15da35d, jack, linux-kernel, syzkaller-bugs, viro Add selftests to verify that deriving namespace file descriptors from pidfd file descriptors works correctly. Signed-off-by: Christian Brauner <brauner@kernel.org> --- .../selftests/pidfd/pidfd_setns_test.c | 258 +++++++++++++++--- 1 file changed, 227 insertions(+), 31 deletions(-) diff --git a/tools/testing/selftests/pidfd/pidfd_setns_test.c b/tools/testing/selftests/pidfd/pidfd_setns_test.c index 47746b0c6acd..7c2a4349170a 100644 --- a/tools/testing/selftests/pidfd/pidfd_setns_test.c +++ b/tools/testing/selftests/pidfd/pidfd_setns_test.c @@ -16,11 +16,56 @@ #include <unistd.h> #include <sys/socket.h> #include <sys/stat.h> +#include <linux/ioctl.h> #include "pidfd.h" #include "../clone3/clone3_selftests.h" #include "../kselftest_harness.h" +#ifndef PIDFS_IOCTL_MAGIC +#define PIDFS_IOCTL_MAGIC 0xFF +#endif + +#ifndef PIDFD_GET_CGROUP_NAMESPACE +#define PIDFD_GET_CGROUP_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 1) +#endif + +#ifndef PIDFD_GET_IPC_NAMESPACE +#define PIDFD_GET_IPC_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 2) +#endif + +#ifndef PIDFD_GET_MNT_NAMESPACE +#define PIDFD_GET_MNT_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 3) +#endif + +#ifndef PIDFD_GET_NET_NAMESPACE +#define PIDFD_GET_NET_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 4) +#endif + +#ifndef PIDFD_GET_PID_NAMESPACE +#define PIDFD_GET_PID_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 5) +#endif + +#ifndef PIDFD_GET_PID_FOR_CHILDREN_NAMESPACE +#define PIDFD_GET_PID_FOR_CHILDREN_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 6) +#endif + +#ifndef PIDFD_GET_TIME_NAMESPACE +#define PIDFD_GET_TIME_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 7) +#endif + +#ifndef PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE +#define PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 8) +#endif + +#ifndef PIDFD_GET_USER_NAMESPACE +#define PIDFD_GET_USER_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 9) +#endif + +#ifndef PIDFD_GET_UTS_NAMESPACE +#define PIDFD_GET_UTS_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 10) +#endif + enum { PIDFD_NS_USER, PIDFD_NS_MNT, @@ -31,22 +76,25 @@ enum { PIDFD_NS_CGROUP, PIDFD_NS_PIDCLD, PIDFD_NS_TIME, + PIDFD_NS_TIMECLD, PIDFD_NS_MAX }; const struct ns_info { const char *name; int flag; + unsigned int pidfd_ioctl; } ns_info[] = { - [PIDFD_NS_USER] = { "user", CLONE_NEWUSER, }, - [PIDFD_NS_MNT] = { "mnt", CLONE_NEWNS, }, - [PIDFD_NS_PID] = { "pid", CLONE_NEWPID, }, - [PIDFD_NS_UTS] = { "uts", CLONE_NEWUTS, }, - [PIDFD_NS_IPC] = { "ipc", CLONE_NEWIPC, }, - [PIDFD_NS_NET] = { "net", CLONE_NEWNET, }, - [PIDFD_NS_CGROUP] = { "cgroup", CLONE_NEWCGROUP, }, - [PIDFD_NS_PIDCLD] = { "pid_for_children", 0, }, - [PIDFD_NS_TIME] = { "time", CLONE_NEWTIME, }, + [PIDFD_NS_USER] = { "user", CLONE_NEWUSER, PIDFD_GET_USER_NAMESPACE, }, + [PIDFD_NS_MNT] = { "mnt", CLONE_NEWNS, PIDFD_GET_MNT_NAMESPACE, }, + [PIDFD_NS_PID] = { "pid", CLONE_NEWPID, PIDFD_GET_PID_NAMESPACE, }, + [PIDFD_NS_UTS] = { "uts", CLONE_NEWUTS, PIDFD_GET_UTS_NAMESPACE, }, + [PIDFD_NS_IPC] = { "ipc", CLONE_NEWIPC, PIDFD_GET_IPC_NAMESPACE, }, + [PIDFD_NS_NET] = { "net", CLONE_NEWNET, PIDFD_GET_NET_NAMESPACE, }, + [PIDFD_NS_CGROUP] = { "cgroup", CLONE_NEWCGROUP, PIDFD_GET_CGROUP_NAMESPACE, }, + [PIDFD_NS_TIME] = { "time", CLONE_NEWTIME, PIDFD_GET_TIME_NAMESPACE, }, + [PIDFD_NS_PIDCLD] = { "pid_for_children", 0, PIDFD_GET_PID_FOR_CHILDREN_NAMESPACE, }, + [PIDFD_NS_TIMECLD] = { "time_for_children", 0, PIDFD_GET_TIME_FOR_CHILDREN_NAMESPACE, }, }; FIXTURE(current_nsset) @@ -54,6 +102,7 @@ FIXTURE(current_nsset) pid_t pid; int pidfd; int nsfds[PIDFD_NS_MAX]; + int child_pidfd_derived_nsfds[PIDFD_NS_MAX]; pid_t child_pid_exited; int child_pidfd_exited; @@ -61,10 +110,12 @@ FIXTURE(current_nsset) pid_t child_pid1; int child_pidfd1; int child_nsfds1[PIDFD_NS_MAX]; + int child_pidfd_derived_nsfds1[PIDFD_NS_MAX]; pid_t child_pid2; int child_pidfd2; int child_nsfds2[PIDFD_NS_MAX]; + int child_pidfd_derived_nsfds2[PIDFD_NS_MAX]; }; static int sys_waitid(int which, pid_t pid, int options) @@ -128,9 +179,12 @@ FIXTURE_SETUP(current_nsset) char c; for (i = 0; i < PIDFD_NS_MAX; i++) { - self->nsfds[i] = -EBADF; - self->child_nsfds1[i] = -EBADF; - self->child_nsfds2[i] = -EBADF; + self->nsfds[i] = -EBADF; + self->child_nsfds1[i] = -EBADF; + self->child_nsfds2[i] = -EBADF; + self->child_pidfd_derived_nsfds[i] = -EBADF; + self->child_pidfd_derived_nsfds1[i] = -EBADF; + self->child_pidfd_derived_nsfds2[i] = -EBADF; } proc_fd = open("/proc/self/ns", O_DIRECTORY | O_CLOEXEC); @@ -139,6 +193,11 @@ FIXTURE_SETUP(current_nsset) } self->pid = getpid(); + self->pidfd = sys_pidfd_open(self->pid, 0); + EXPECT_GT(self->pidfd, 0) { + TH_LOG("%m - Failed to open pidfd for process %d", self->pid); + } + for (i = 0; i < PIDFD_NS_MAX; i++) { const struct ns_info *info = &ns_info[i]; self->nsfds[i] = openat(proc_fd, info->name, O_RDONLY | O_CLOEXEC); @@ -148,20 +207,27 @@ FIXTURE_SETUP(current_nsset) info->name, self->pid); } } - } - self->pidfd = sys_pidfd_open(self->pid, 0); - EXPECT_GT(self->pidfd, 0) { - TH_LOG("%m - Failed to open pidfd for process %d", self->pid); + self->child_pidfd_derived_nsfds[i] = ioctl(self->pidfd, info->pidfd_ioctl, 0); + if (self->child_pidfd_derived_nsfds[i] < 0) { + EXPECT_EQ(errno, EOPNOTSUPP) { + TH_LOG("%m - Failed to derive %s namespace from pidfd of process %d", + info->name, self->pid); + } + } } /* Create task that exits right away. */ - self->child_pid_exited = create_child(&self->child_pidfd_exited, - CLONE_NEWUSER | CLONE_NEWNET); + self->child_pid_exited = create_child(&self->child_pidfd_exited, 0); EXPECT_GE(self->child_pid_exited, 0); - if (self->child_pid_exited == 0) + if (self->child_pid_exited == 0) { + if (self->nsfds[PIDFD_NS_USER] >= 0 && unshare(CLONE_NEWUSER) < 0) + _exit(EXIT_FAILURE); + if (self->nsfds[PIDFD_NS_NET] >= 0 && unshare(CLONE_NEWNET) < 0) + _exit(EXIT_FAILURE); _exit(EXIT_SUCCESS); + } ASSERT_EQ(sys_waitid(P_PID, self->child_pid_exited, WEXITED | WNOWAIT), 0); @@ -174,18 +240,43 @@ FIXTURE_SETUP(current_nsset) EXPECT_EQ(ret, 0); /* Create tasks that will be stopped. */ - self->child_pid1 = create_child(&self->child_pidfd1, - CLONE_NEWUSER | CLONE_NEWNS | - CLONE_NEWCGROUP | CLONE_NEWIPC | - CLONE_NEWUTS | CLONE_NEWPID | - CLONE_NEWNET); + if (self->nsfds[PIDFD_NS_USER] >= 0 && self->nsfds[PIDFD_NS_PID] >= 0) + self->child_pid1 = create_child(&self->child_pidfd1, CLONE_NEWUSER | CLONE_NEWPID); + else if (self->nsfds[PIDFD_NS_PID] >= 0) + self->child_pid1 = create_child(&self->child_pidfd1, CLONE_NEWPID); + else if (self->nsfds[PIDFD_NS_USER] >= 0) + self->child_pid1 = create_child(&self->child_pidfd1, CLONE_NEWUSER); + else + self->child_pid1 = create_child(&self->child_pidfd1, 0); EXPECT_GE(self->child_pid1, 0); if (self->child_pid1 == 0) { close(ipc_sockets[0]); - if (!switch_timens()) + if (self->nsfds[PIDFD_NS_MNT] >= 0 && unshare(CLONE_NEWNS) < 0) { + TH_LOG("%m - Failed to unshare mount namespace for process %d", self->pid); _exit(EXIT_FAILURE); + } + if (self->nsfds[PIDFD_NS_CGROUP] >= 0 && unshare(CLONE_NEWCGROUP) < 0) { + TH_LOG("%m - Failed to unshare cgroup namespace for process %d", self->pid); + _exit(EXIT_FAILURE); + } + if (self->nsfds[PIDFD_NS_IPC] >= 0 && unshare(CLONE_NEWIPC) < 0) { + TH_LOG("%m - Failed to unshare ipc namespace for process %d", self->pid); + _exit(EXIT_FAILURE); + } + if (self->nsfds[PIDFD_NS_UTS] >= 0 && unshare(CLONE_NEWUTS) < 0) { + TH_LOG("%m - Failed to unshare uts namespace for process %d", self->pid); + _exit(EXIT_FAILURE); + } + if (self->nsfds[PIDFD_NS_NET] >= 0 && unshare(CLONE_NEWNET) < 0) { + TH_LOG("%m - Failed to unshare net namespace for process %d", self->pid); + _exit(EXIT_FAILURE); + } + if (self->nsfds[PIDFD_NS_TIME] >= 0 && !switch_timens()) { + TH_LOG("%m - Failed to unshare time namespace for process %d", self->pid); + _exit(EXIT_FAILURE); + } if (write_nointr(ipc_sockets[1], "1", 1) < 0) _exit(EXIT_FAILURE); @@ -203,18 +294,43 @@ FIXTURE_SETUP(current_nsset) ret = socketpair(AF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets); EXPECT_EQ(ret, 0); - self->child_pid2 = create_child(&self->child_pidfd2, - CLONE_NEWUSER | CLONE_NEWNS | - CLONE_NEWCGROUP | CLONE_NEWIPC | - CLONE_NEWUTS | CLONE_NEWPID | - CLONE_NEWNET); + if (self->nsfds[PIDFD_NS_USER] >= 0 && self->nsfds[PIDFD_NS_PID] >= 0) + self->child_pid2 = create_child(&self->child_pidfd2, CLONE_NEWUSER | CLONE_NEWPID); + else if (self->nsfds[PIDFD_NS_PID] >= 0) + self->child_pid2 = create_child(&self->child_pidfd2, CLONE_NEWPID); + else if (self->nsfds[PIDFD_NS_USER] >= 0) + self->child_pid2 = create_child(&self->child_pidfd2, CLONE_NEWUSER); + else + self->child_pid2 = create_child(&self->child_pidfd2, 0); EXPECT_GE(self->child_pid2, 0); if (self->child_pid2 == 0) { close(ipc_sockets[0]); - if (!switch_timens()) + if (self->nsfds[PIDFD_NS_MNT] >= 0 && unshare(CLONE_NEWNS) < 0) { + TH_LOG("%m - Failed to unshare mount namespace for process %d", self->pid); + _exit(EXIT_FAILURE); + } + if (self->nsfds[PIDFD_NS_CGROUP] >= 0 && unshare(CLONE_NEWCGROUP) < 0) { + TH_LOG("%m - Failed to unshare cgroup namespace for process %d", self->pid); _exit(EXIT_FAILURE); + } + if (self->nsfds[PIDFD_NS_IPC] >= 0 && unshare(CLONE_NEWIPC) < 0) { + TH_LOG("%m - Failed to unshare ipc namespace for process %d", self->pid); + _exit(EXIT_FAILURE); + } + if (self->nsfds[PIDFD_NS_UTS] >= 0 && unshare(CLONE_NEWUTS) < 0) { + TH_LOG("%m - Failed to unshare uts namespace for process %d", self->pid); + _exit(EXIT_FAILURE); + } + if (self->nsfds[PIDFD_NS_NET] >= 0 && unshare(CLONE_NEWNET) < 0) { + TH_LOG("%m - Failed to unshare net namespace for process %d", self->pid); + _exit(EXIT_FAILURE); + } + if (self->nsfds[PIDFD_NS_TIME] >= 0 && !switch_timens()) { + TH_LOG("%m - Failed to unshare time namespace for process %d", self->pid); + _exit(EXIT_FAILURE); + } if (write_nointr(ipc_sockets[1], "1", 1) < 0) _exit(EXIT_FAILURE); @@ -267,6 +383,22 @@ FIXTURE_SETUP(current_nsset) info->name, self->child_pid1); } } + + self->child_pidfd_derived_nsfds1[i] = ioctl(self->child_pidfd1, info->pidfd_ioctl, 0); + if (self->child_pidfd_derived_nsfds1[i] < 0) { + EXPECT_EQ(errno, EOPNOTSUPP) { + TH_LOG("%m - Failed to derive %s namespace from pidfd of process %d", + info->name, self->child_pid1); + } + } + + self->child_pidfd_derived_nsfds2[i] = ioctl(self->child_pidfd2, info->pidfd_ioctl, 0); + if (self->child_pidfd_derived_nsfds2[i] < 0) { + EXPECT_EQ(errno, EOPNOTSUPP) { + TH_LOG("%m - Failed to derive %s namespace from pidfd of process %d", + info->name, self->child_pid2); + } + } } close(proc_fd); @@ -288,6 +420,12 @@ FIXTURE_TEARDOWN(current_nsset) close(self->child_nsfds1[i]); if (self->child_nsfds2[i] >= 0) close(self->child_nsfds2[i]); + if (self->child_pidfd_derived_nsfds[i] >= 0) + close(self->child_pidfd_derived_nsfds[i]); + if (self->child_pidfd_derived_nsfds1[i] >= 0) + close(self->child_pidfd_derived_nsfds1[i]); + if (self->child_pidfd_derived_nsfds2[i] >= 0) + close(self->child_pidfd_derived_nsfds2[i]); } if (self->child_pidfd1 >= 0) @@ -446,6 +584,42 @@ TEST_F(current_nsset, nsfd_incremental_setns) } } +TEST_F(current_nsset, pidfd_derived_nsfd_incremental_setns) +{ + int i; + pid_t pid; + + pid = getpid(); + for (i = 0; i < PIDFD_NS_MAX; i++) { + const struct ns_info *info = &ns_info[i]; + int nsfd; + + if (self->child_pidfd_derived_nsfds1[i] < 0) + continue; + + if (info->flag) { + ASSERT_EQ(setns(self->child_pidfd_derived_nsfds1[i], info->flag), 0) { + TH_LOG("%m - Failed to setns to %s namespace of %d via nsfd %d", + info->name, self->child_pid1, + self->child_pidfd_derived_nsfds1[i]); + } + } + + /* Verify that we have changed to the correct namespaces. */ + if (info->flag == CLONE_NEWPID) + nsfd = self->child_pidfd_derived_nsfds[i]; + else + nsfd = self->child_pidfd_derived_nsfds1[i]; + ASSERT_EQ(in_same_namespace(nsfd, pid, info->name), 1) { + TH_LOG("setns failed to place us correctly into %s namespace of %d via nsfd %d", + info->name, self->child_pid1, + self->child_pidfd_derived_nsfds1[i]); + } + TH_LOG("Managed to correctly setns to %s namespace of %d via nsfd %d", + info->name, self->child_pid1, self->child_pidfd_derived_nsfds1[i]); + } +} + TEST_F(current_nsset, pidfd_one_shot_setns) { unsigned flags = 0; @@ -542,6 +716,28 @@ TEST_F(current_nsset, no_foul_play) info->name, self->child_pid2, self->child_nsfds2[i]); } + + /* + * Can't setns to a user namespace outside of our hierarchy since we + * don't have caps in there and didn't create it. That means that under + * no circumstances should we be able to setns to any of the other + * ones since they aren't owned by our user namespace. + */ + for (i = 0; i < PIDFD_NS_MAX; i++) { + const struct ns_info *info = &ns_info[i]; + + if (self->child_pidfd_derived_nsfds2[i] < 0 || !info->flag) + continue; + + ASSERT_NE(setns(self->child_pidfd_derived_nsfds2[i], info->flag), 0) { + TH_LOG("Managed to setns to %s namespace of %d via nsfd %d", + info->name, self->child_pid2, + self->child_pidfd_derived_nsfds2[i]); + } + TH_LOG("%m - Correctly failed to setns to %s namespace of %d via nsfd %d", + info->name, self->child_pid2, + self->child_pidfd_derived_nsfds2[i]); + } } TEST(setns_einval) -- 2.43.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-07-22 13:14 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-20 16:36 [syzbot] [fs?] BUG: unable to handle kernel NULL pointer dereference in path_from_stashed syzbot 2024-07-21 4:50 ` Edward Adam Davis 2024-07-21 4:57 ` syzbot 2024-07-21 5:14 ` Edward Adam Davis 2024-07-21 5:41 ` syzbot 2024-07-21 6:23 ` [PATCH] fs/pidfs: when time ns disabled add check for ioctl Edward Adam Davis 2024-07-21 10:59 ` Matthew Wilcox 2024-07-22 8:00 ` Christian Brauner 2024-07-22 13:13 ` [PATCH 1/2] pidfs: handle kernels without namespaces cleanly Christian Brauner 2024-07-22 13:13 ` [PATCH 2/2] pidfs: add selftests for new namespace ioctls Christian Brauner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.