[PATCH v1] seccomp: Release filter when copy_process() fails.

From: Kuniyuki Iwashima
Date: Mon Aug 22 2022 - 16:45:53 EST

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 ref count.

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, let's call seccomp_filter_release() in copy_process()
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: syzbot+ab17848fe269b573eb71@xxxxxxxxxxxxxxxxxxxxxxxxx
Reported-by: Ayushman Dutta <ayudutta@xxxxxxxxxx>
Signed-off-by: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>
---
kernel/fork.c | 2 ++
kernel/seccomp.c | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 90c85b17bf69..031290fc527c 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);

@@ -2498,6 +2499,7 @@ static __latent_entropy struct task_struct *copy_process(
sched_core_free(p);
spin_unlock(&current->sighand->siglock);
write_unlock_irq(&tasklist_lock);
+ seccomp_filter_release(p);
cgroup_cancel_fork(p, args);
bad_fork_put_pidfd:
if (clone_flags & CLONE_PIDFD) {
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index e9852d1b4a5e..017c1dd5b0d4 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -563,8 +563,6 @@ void seccomp_filter_release(struct task_struct *tsk)
{
struct seccomp_filter *orig = tsk->seccomp.filter;

- /* We are effectively holding the siglock by not having any sighand. */
- WARN_ON(tsk->sighand != NULL);

/* Detach task from its filter tree. */
tsk->seccomp.filter = NULL;
--
2.30.2