Our syzbot instance reported memory leaks in do_seccomp() [0], similar
to the report [1]. It shows that we miss freeing struct seccomp_filter
and some objects included in it.
We can reproduce the issue with the program below [2] which calls one
seccomp() and two clone() syscalls.
The first clone()d child exits earlier than its parent and sends a
signal to kill it during the second clone(), more precisely before the
fatal_signal_pending() test in copy_process(). When the parent receives
the signal, it has to destroy the embryonic process and return -EINTR to
user space. In the failure path, we have to call seccomp_filter_release()
to decrement the filter's refcount.
Initially, we called it in free_task() called from the failure path, but
the commit 3a15fb6ed92c ("seccomp: release filter after task is fully
dead") moved it to release_task() to notify user space as early as possible
that the filter is no longer used.
To keep the change and current seccomp refcount semantics, let's move
copy_seccomp() just after the signal check and add a WARN_ON_ONCE() in
free_task() for future debugging.
[0]:
unreferenced object 0xffff8880063add00 (size 256):
comm "repro_seccomp", pid 230, jiffies 4294687090 (age 9.914s)
hex dump (first 32 bytes):
01 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 ................
ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
backtrace:
do_seccomp (./include/linux/slab.h:600 ./include/linux/slab.h:733 kernel/seccomp.c:666 kernel/seccomp.c:708 kernel/seccomp.c:1871 kernel/seccomp.c:1991)
do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
unreferenced object 0xffffc90000035000 (size 4096):
comm "repro_seccomp", pid 230, jiffies 4294687090 (age 9.915s)
hex dump (first 32 bytes):
01 00 00 00 00 00 00 00 00 00 00 00 05 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
__vmalloc_node_range (mm/vmalloc.c:3226)
__vmalloc_node (mm/vmalloc.c:3261 (discriminator 4))
bpf_prog_alloc_no_stats (kernel/bpf/core.c:91)
bpf_prog_alloc (kernel/bpf/core.c:129)
bpf_prog_create_from_user (net/core/filter.c:1414)
do_seccomp (kernel/seccomp.c:671 kernel/seccomp.c:708 kernel/seccomp.c:1871 kernel/seccomp.c:1991)
do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
unreferenced object 0xffff888003fa1000 (size 1024):
comm "repro_seccomp", pid 230, jiffies 4294687090 (age 9.915s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
bpf_prog_alloc_no_stats (./include/linux/slab.h:600 ./include/linux/slab.h:733 kernel/bpf/core.c:95)
bpf_prog_alloc (kernel/bpf/core.c:129)
bpf_prog_create_from_user (net/core/filter.c:1414)
do_seccomp (kernel/seccomp.c:671 kernel/seccomp.c:708 kernel/seccomp.c:1871 kernel/seccomp.c:1991)
do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
unreferenced object 0xffff888006360240 (size 16):
comm "repro_seccomp", pid 230, jiffies 4294687090 (age 9.915s)
hex dump (first 16 bytes):
01 00 37 00 76 65 72 6c e0 83 01 06 80 88 ff ff ..7.verl........
backtrace:
bpf_prog_store_orig_filter (net/core/filter.c:1137)
bpf_prog_create_from_user (net/core/filter.c:1428)
do_seccomp (kernel/seccomp.c:671 kernel/seccomp.c:708 kernel/seccomp.c:1871 kernel/seccomp.c:1991)
do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
unreferenced object 0xffff8880060183e0 (size 8):
comm "repro_seccomp", pid 230, jiffies 4294687090 (age 9.915s)
hex dump (first 8 bytes):
06 00 00 00 00 00 ff 7f ........
backtrace:
kmemdup (mm/util.c:129)
bpf_prog_store_orig_filter (net/core/filter.c:1144)
bpf_prog_create_from_user (net/core/filter.c:1428)
do_seccomp (kernel/seccomp.c:671 kernel/seccomp.c:708 kernel/seccomp.c:1871 kernel/seccomp.c:1991)
do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
[1]: https://syzkaller.appspot.com/bug?id=2809bb0ac77ad9aa3f4afe42d6a610aba594a987
[2]:
#define _GNU_SOURCE
#include <sched.h>
#include <signal.h>
#include <unistd.h>
#include <sys/syscall.h>
#include <linux/filter.h>
#include <linux/seccomp.h>
void main(void)
{
struct sock_filter filter[] = {
BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW),
};
struct sock_fprog fprog = {
.len = sizeof(filter) / sizeof(filter[0]),
.filter = filter,
};
long i, pid;
syscall(__NR_seccomp, SECCOMP_SET_MODE_FILTER, 0, &fprog);
for (i = 0; i < 2; i++) {
pid = syscall(__NR_clone, CLONE_NEWNET | SIGKILL, NULL, NULL, 0);
if (pid == 0)
return;
}
}
Fixes: 3a15fb6ed92c ("seccomp: release filter after task is fully dead")
Reported-by: [email protected]
Reported-by: Ayushman Dutta <[email protected]>
Suggested-by: Kees Cook <[email protected]>
Signed-off-by: Kuniyuki Iwashima <[email protected]>
---
v2:
* Move copy_seccomp() after no failure path instead of adding
seccomp_filter_release() in the failure path.
v1: https://lore.kernel.org/lkml/[email protected]/
---
kernel/fork.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/kernel/fork.c b/kernel/fork.c
index 90c85b17bf69..21c7365c8322 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -537,6 +537,7 @@ void put_task_stack(struct task_struct *tsk)
void free_task(struct task_struct *tsk)
{
+ WARN_ON_ONCE(tsk->seccomp.filter);
release_user_cpus_ptr(tsk);
scs_release(tsk);
@@ -2409,12 +2410,6 @@ static __latent_entropy struct task_struct *copy_process(
spin_lock(¤t->sighand->siglock);
- /*
- * Copy seccomp details explicitly here, in case they were changed
- * before holding sighand lock.
- */
- copy_seccomp(p);
-
rv_task_fork(p);
rseq_fork(p, clone_flags);
@@ -2431,6 +2426,14 @@ static __latent_entropy struct task_struct *copy_process(
goto bad_fork_cancel_cgroup;
}
+ /* No more failure paths after this point. */
+
+ /*
+ * Copy seccomp details explicitly here, in case they were changed
+ * before holding sighand lock.
+ */
+ copy_seccomp(p);
+
init_task_pid_links(p);
if (likely(p->pid)) {
ptrace_init_task(p, (clone_flags & CLONE_PTRACE) || trace);
--
2.30.2
On Mon, Aug 22, 2022 at 05:48:06PM -0700, Kuniyuki Iwashima wrote:
> Our syzbot instance reported memory leaks in do_seccomp() [0], similar
> to the report [1]. It shows that we miss freeing struct seccomp_filter
> and some objects included in it.
>
> We can reproduce the issue with the program below [2] which calls one
> seccomp() and two clone() syscalls.
>
> The first clone()d child exits earlier than its parent and sends a
> signal to kill it during the second clone(), more precisely before the
> fatal_signal_pending() test in copy_process(). When the parent receives
> the signal, it has to destroy the embryonic process and return -EINTR to
> user space. In the failure path, we have to call seccomp_filter_release()
> to decrement the filter's refcount.
>
> Initially, we called it in free_task() called from the failure path, but
> the commit 3a15fb6ed92c ("seccomp: release filter after task is fully
> dead") moved it to release_task() to notify user space as early as possible
> that the filter is no longer used.
>
> To keep the change and current seccomp refcount semantics, let's move
> copy_seccomp() just after the signal check and add a WARN_ON_ONCE() in
> free_task() for future debugging.
>
> [0]:
> unreferenced object 0xffff8880063add00 (size 256):
> comm "repro_seccomp", pid 230, jiffies 4294687090 (age 9.914s)
> hex dump (first 32 bytes):
> 01 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 ................
> ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
> backtrace:
> do_seccomp (./include/linux/slab.h:600 ./include/linux/slab.h:733 kernel/seccomp.c:666 kernel/seccomp.c:708 kernel/seccomp.c:1871 kernel/seccomp.c:1991)
> do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
> unreferenced object 0xffffc90000035000 (size 4096):
> comm "repro_seccomp", pid 230, jiffies 4294687090 (age 9.915s)
> hex dump (first 32 bytes):
> 01 00 00 00 00 00 00 00 00 00 00 00 05 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> __vmalloc_node_range (mm/vmalloc.c:3226)
> __vmalloc_node (mm/vmalloc.c:3261 (discriminator 4))
> bpf_prog_alloc_no_stats (kernel/bpf/core.c:91)
> bpf_prog_alloc (kernel/bpf/core.c:129)
> bpf_prog_create_from_user (net/core/filter.c:1414)
> do_seccomp (kernel/seccomp.c:671 kernel/seccomp.c:708 kernel/seccomp.c:1871 kernel/seccomp.c:1991)
> do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
> unreferenced object 0xffff888003fa1000 (size 1024):
> comm "repro_seccomp", pid 230, jiffies 4294687090 (age 9.915s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> bpf_prog_alloc_no_stats (./include/linux/slab.h:600 ./include/linux/slab.h:733 kernel/bpf/core.c:95)
> bpf_prog_alloc (kernel/bpf/core.c:129)
> bpf_prog_create_from_user (net/core/filter.c:1414)
> do_seccomp (kernel/seccomp.c:671 kernel/seccomp.c:708 kernel/seccomp.c:1871 kernel/seccomp.c:1991)
> do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
> unreferenced object 0xffff888006360240 (size 16):
> comm "repro_seccomp", pid 230, jiffies 4294687090 (age 9.915s)
> hex dump (first 16 bytes):
> 01 00 37 00 76 65 72 6c e0 83 01 06 80 88 ff ff ..7.verl........
> backtrace:
> bpf_prog_store_orig_filter (net/core/filter.c:1137)
> bpf_prog_create_from_user (net/core/filter.c:1428)
> do_seccomp (kernel/seccomp.c:671 kernel/seccomp.c:708 kernel/seccomp.c:1871 kernel/seccomp.c:1991)
> do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
> unreferenced object 0xffff8880060183e0 (size 8):
> comm "repro_seccomp", pid 230, jiffies 4294687090 (age 9.915s)
> hex dump (first 8 bytes):
> 06 00 00 00 00 00 ff 7f ........
> backtrace:
> kmemdup (mm/util.c:129)
> bpf_prog_store_orig_filter (net/core/filter.c:1144)
> bpf_prog_create_from_user (net/core/filter.c:1428)
> do_seccomp (kernel/seccomp.c:671 kernel/seccomp.c:708 kernel/seccomp.c:1871 kernel/seccomp.c:1991)
> do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
>
> [1]: https://syzkaller.appspot.com/bug?id=2809bb0ac77ad9aa3f4afe42d6a610aba594a987
>
> [2]:
> #define _GNU_SOURCE
> #include <sched.h>
> #include <signal.h>
> #include <unistd.h>
> #include <sys/syscall.h>
> #include <linux/filter.h>
> #include <linux/seccomp.h>
>
> void main(void)
> {
> struct sock_filter filter[] = {
> BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW),
> };
> struct sock_fprog fprog = {
> .len = sizeof(filter) / sizeof(filter[0]),
> .filter = filter,
> };
> long i, pid;
>
> syscall(__NR_seccomp, SECCOMP_SET_MODE_FILTER, 0, &fprog);
>
> for (i = 0; i < 2; i++) {
> pid = syscall(__NR_clone, CLONE_NEWNET | SIGKILL, NULL, NULL, 0);
> if (pid == 0)
> return;
> }
> }
>
> Fixes: 3a15fb6ed92c ("seccomp: release filter after task is fully dead")
> Reported-by: [email protected]
> Reported-by: Ayushman Dutta <[email protected]>
> Suggested-by: Kees Cook <[email protected]>
> Signed-off-by: Kuniyuki Iwashima <[email protected]>
> ---
Looks good,
Reviewed-by: Christian Brauner (Microsoft) <[email protected]>
Hi Kuniyuki,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.0-rc2 next-20220823]
[cannot apply to kees/for-next/pstore]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/seccomp-Move-copy_seccomp-to-no-failure-path/20220823-085102
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 072e51356cd5a4a1c12c1020bc054c99b98333df
config: x86_64-randconfig-a002-20220822 (https://download.01.org/0day-ci/archive/20220823/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f5034318d40caefe1d5445e4ae3db7b632010bb8
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kuniyuki-Iwashima/seccomp-Move-copy_seccomp-to-no-failure-path/20220823-085102
git checkout f5034318d40caefe1d5445e4ae3db7b632010bb8
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> kernel/fork.c:540:28: error: no member named 'filter' in 'struct seccomp'
WARN_ON_ONCE(tsk->seccomp.filter);
~~~~~~~~~~~~ ^
include/asm-generic/bug.h:110:25: note: expanded from macro 'WARN_ON_ONCE'
int __ret_warn_on = !!(condition); \
^~~~~~~~~
1 error generated.
vim +540 kernel/fork.c
537
538 void free_task(struct task_struct *tsk)
539 {
> 540 WARN_ON_ONCE(tsk->seccomp.filter);
541 release_user_cpus_ptr(tsk);
542 scs_release(tsk);
543
--
0-DAY CI Kernel Test Service
https://01.org/lkp