From 9defeeaf093a169645cacf6a017c6b3609e0c763 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Mon, 5 Feb 2024 21:06:14 -0800 Subject: [PATCH] `seccomp`: Check that programs that are too large are rejected. This is good for three reasons: - Linux also rejects programs larger than 4,096 (`BPF_MAXINSNS`) instructions. - This avoids making an allocation of unbounded length on the next line. - This avoids the pitfall where gVisor may spend CPU doing BPF bytecode optimizations, which can be worse than O(n), on a program which is unboundedly large. By checking the size of the BPF program before applying bytecode optimizations, this DoS vector is nullified. PiperOrigin-RevId: 604523997 --- pkg/sentry/syscalls/linux/sys_seccomp.go | 6 ++ test/syscalls/linux/seccomp.cc | 107 ++++++++++++++--------- 2 files changed, 73 insertions(+), 40 deletions(-) diff --git a/pkg/sentry/syscalls/linux/sys_seccomp.go b/pkg/sentry/syscalls/linux/sys_seccomp.go index 71cbbbf6c..bee6ebe92 100644 --- a/pkg/sentry/syscalls/linux/sys_seccomp.go +++ b/pkg/sentry/syscalls/linux/sys_seccomp.go @@ -59,6 +59,12 @@ func seccomp(t *kernel.Task, mode, flags uint64, addr hostarch.Addr) error { if _, err := fprog.CopyIn(t, addr); err != nil { return err } + if fprog.Len == 0 || fprog.Len > bpf.MaxInstructions { + // If the filter is already over the maximum number of instructions, + // do not go further and attempt to optimize the bytecode to make it + // smaller. + return linuxerr.EINVAL + } filter := make([]linux.BPFInstruction, int(fprog.Len)) if _, err := linux.CopyBPFInstructionSliceIn(t, hostarch.Addr(fprog.Filter), filter); err != nil { return err diff --git a/test/syscalls/linux/seccomp.cc b/test/syscalls/linux/seccomp.cc index 5ba16a4a6..5dfef9961 100644 --- a/test/syscalls/linux/seccomp.cc +++ b/test/syscalls/linux/seccomp.cc @@ -71,27 +71,27 @@ void ApplySeccompFilter(uint32_t sysno, uint32_t filtered_result, MaybeSave(); struct sock_filter filter[] = { - // A = seccomp_data.arch - BPF_STMT(BPF_LD | BPF_ABS | BPF_W, 4), + // A = seccomp_data.arch + BPF_STMT(BPF_LD | BPF_ABS | BPF_W, 4), #if defined(__x86_64__) - // if (A != AUDIT_ARCH_X86_64) goto kill - BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, AUDIT_ARCH_X86_64, 0, 4), + // if (A != AUDIT_ARCH_X86_64) goto kill + BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, AUDIT_ARCH_X86_64, 0, 4), #elif defined(__aarch64__) - // if (A != AUDIT_ARCH_AARCH64) goto kill - BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, AUDIT_ARCH_AARCH64, 0, 4), + // if (A != AUDIT_ARCH_AARCH64) goto kill + BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, AUDIT_ARCH_AARCH64, 0, 4), #else #error "Unknown architecture" #endif - // A = seccomp_data.nr - BPF_STMT(BPF_LD | BPF_ABS | BPF_W, 0), - // if (A != sysno) goto allow - BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, sysno, 0, 1), - // return filtered_result - BPF_STMT(BPF_RET | BPF_K, filtered_result), - // allow: return SECCOMP_RET_ALLOW - BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW), - // kill: return SECCOMP_RET_KILL - BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_KILL), + // A = seccomp_data.nr + BPF_STMT(BPF_LD | BPF_ABS | BPF_W, 0), + // if (A != sysno) goto allow + BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, sysno, 0, 1), + // return filtered_result + BPF_STMT(BPF_RET | BPF_K, filtered_result), + // allow: return SECCOMP_RET_ALLOW + BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW), + // kill: return SECCOMP_RET_KILL + BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_KILL), }; struct sock_fprog prog; prog.len = ABSL_ARRAYSIZE(filter); @@ -119,27 +119,27 @@ void ApplyUncacheableFilter(uint32_t sysno) { MaybeSave(); struct sock_filter filter[] = { - // A = seccomp_data.arch - BPF_STMT(BPF_LD | BPF_ABS | BPF_W, 4), + // A = seccomp_data.arch + BPF_STMT(BPF_LD | BPF_ABS | BPF_W, 4), #if defined(__x86_64__) - // if (A != AUDIT_ARCH_X86_64) goto kill - BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, AUDIT_ARCH_X86_64, 0, 4), + // if (A != AUDIT_ARCH_X86_64) goto kill + BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, AUDIT_ARCH_X86_64, 0, 4), #elif defined(__aarch64__) - // if (A != AUDIT_ARCH_AARCH64) goto kill - BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, AUDIT_ARCH_AARCH64, 0, 4), + // if (A != AUDIT_ARCH_AARCH64) goto kill + BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, AUDIT_ARCH_AARCH64, 0, 4), #else #error "Unknown architecture" #endif - // A = seccomp_data.nr - BPF_STMT(BPF_LD | BPF_ABS | BPF_W, 0), - // if (A != sysno) goto end - BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, sysno, 0, 1), - // A = seccomp_data.args[0] - BPF_STMT(BPF_LD | BPF_ABS | BPF_W, 16), - // end: return SECCOMP_RET_ALLOW - BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW), - // kill: return SECCOMP_RET_KILL - BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_KILL), + // A = seccomp_data.nr + BPF_STMT(BPF_LD | BPF_ABS | BPF_W, 0), + // if (A != sysno) goto end + BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, sysno, 0, 1), + // A = seccomp_data.args[0] + BPF_STMT(BPF_LD | BPF_ABS | BPF_W, 16), + // end: return SECCOMP_RET_ALLOW + BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW), + // kill: return SECCOMP_RET_KILL + BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_KILL), }; struct sock_fprog prog; prog.len = ABSL_ARRAYSIZE(filter); @@ -169,8 +169,7 @@ TEST(SeccompTest, RetKillCausesDeathBySIGSYS) { pid_t const pid = fork(); if (pid == 0) { // Register a signal handler for SIGSYS that we don't expect to be invoked. - RegisterSignalHandler( - SIGSYS, +[](int, siginfo_t*, void*) { _exit(1); }); + RegisterSignalHandler(SIGSYS, +[](int, siginfo_t*, void*) { _exit(1); }); ApplySeccompFilter(kFilteredSyscall, SECCOMP_RET_KILL); syscall(kFilteredSyscall); TEST_CHECK_MSG(false, "Survived invocation of test syscall"); @@ -189,8 +188,7 @@ TEST(SeccompTest, RetKillOnlyKillsOneThread) { pid_t const pid = fork(); if (pid == 0) { // Register a signal handler for SIGSYS that we don't expect to be invoked. - RegisterSignalHandler( - SIGSYS, +[](int, siginfo_t*, void*) { _exit(1); }); + RegisterSignalHandler(SIGSYS, +[](int, siginfo_t*, void*) { _exit(1); }); ApplySeccompFilter(kFilteredSyscall, SECCOMP_RET_KILL); // Pass CLONE_VFORK to block the original thread in the child process until // the clone thread exits with SIGSYS. @@ -296,8 +294,7 @@ TEST(SeccompTest, RetKillVsyscallCausesDeathBySIGSYS) { pid_t const pid = fork(); if (pid == 0) { // Register a signal handler for SIGSYS that we don't expect to be invoked. - RegisterSignalHandler( - SIGSYS, +[](int, siginfo_t*, void*) { _exit(1); }); + RegisterSignalHandler(SIGSYS, +[](int, siginfo_t*, void*) { _exit(1); }); ApplySeccompFilter(SYS_time, SECCOMP_RET_KILL); vsyscall_time(nullptr); // Should result in death. TEST_CHECK_MSG(false, "Survived invocation of test syscall"); @@ -424,8 +421,7 @@ TEST(SeccompTest, LeastPermissiveFilterReturnValueApplies) { // one that causes the kill that should be ignored. pid_t const pid = fork(); if (pid == 0) { - RegisterSignalHandler( - SIGSYS, +[](int, siginfo_t*, void*) { _exit(1); }); + RegisterSignalHandler(SIGSYS, +[](int, siginfo_t*, void*) { _exit(1); }); ApplySeccompFilter(kFilteredSyscall, SECCOMP_RET_TRACE); ApplySeccompFilter(kFilteredSyscall, SECCOMP_RET_KILL); ApplySeccompFilter(kFilteredSyscall, SECCOMP_RET_ERRNO | ENOTNAM); @@ -469,6 +465,37 @@ TEST(SeccompTest, FiltersPreservedAcrossForkAndExecve) { << "status " << status; } +TEST(SeccompTest, EmptyProgramIsRejected) { + struct sock_fprog prog; + prog.len = 0; + prog.filter = nullptr; + ASSERT_THAT(syscall(__NR_seccomp, SECCOMP_MODE_FILTER, &prog), + SyscallFailsWithErrno(EINVAL)); + MaybeSave(); + ASSERT_THAT(prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog, 0, 0), + SyscallFailsWithErrno(EINVAL)); +} + +TEST(SeccompTest, ProgramTooLargeIsRejected) { + constexpr int kTooLargeFilterSize = 4097; // BPF_MAXINSNS + 1 + TEST_PCHECK(prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == 0); + MaybeSave(); + struct sock_filter filter[kTooLargeFilterSize]; + for (int i = 0; i < kTooLargeFilterSize; ++i) { + filter[i] = BPF_STMT(BPF_LD | BPF_ABS | BPF_W, 0); // A = seccomp_data.nr + } + filter[kTooLargeFilterSize - 1] = + BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW); // Return allow + struct sock_fprog prog; + prog.len = ABSL_ARRAYSIZE(filter); + prog.filter = filter; + ASSERT_THAT(syscall(__NR_seccomp, SECCOMP_MODE_FILTER, &prog), + SyscallFailsWithErrno(EINVAL)); + MaybeSave(); + ASSERT_THAT(prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog, 0, 0), + SyscallFailsWithErrno(EINVAL)); +} + } // namespace } // namespace testing