ACK: [B][C][SRU][PATCH 1/1] selftests/seccomp: Enhance per-arch ptrace syscall skip tests
Stefan Bader
stefan.bader at canonical.com
Wed May 29 14:15:18 UTC 2019
On 16.05.19 12:20, Po-Hsu Lin wrote:
> From: Kees Cook <keescook at chromium.org>
>
> BugLink: https://bugs.launchpad.net/bugs/1812796
>
> Passing EPERM during syscall skipping was confusing since the test wasn't
> actually exercising the errno evaluation -- it was just passing a literal
> "1" (EPERM). Instead, expand the tests to check both direct value returns
> (positive, 45000 in this case), and errno values (negative, -ESRCH in this
> case) to check both fake success and fake failure during syscall skipping.
>
> Reported-by: Colin Ian King <colin.king at canonical.com>
> Fixes: a33b2d0359a0 ("selftests/seccomp: Add tests for basic ptrace actions")
> Cc: stable at vger.kernel.org
> Signed-off-by: Kees Cook <keescook at chromium.org>
> Signed-off-by: Shuah Khan <shuah at kernel.org>
> (cherry picked from commit ed5f13261cb65b02c611ae9971677f33581d4286)
> Signed-off-by: Po-Hsu Lin <po-hsu.lin at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---
> tools/testing/selftests/seccomp/seccomp_bpf.c | 72 +++++++++++++++++++++------
> 1 file changed, 57 insertions(+), 15 deletions(-)
>
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index e350cf3..292bbf0 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -1563,7 +1563,16 @@ TEST_F(TRACE_poke, getpid_runs_normally)
> #ifdef SYSCALL_NUM_RET_SHARE_REG
> # define EXPECT_SYSCALL_RETURN(val, action) EXPECT_EQ(-1, action)
> #else
> -# define EXPECT_SYSCALL_RETURN(val, action) EXPECT_EQ(val, action)
> +# define EXPECT_SYSCALL_RETURN(val, action) \
> + do { \
> + errno = 0; \
> + if (val < 0) { \
> + EXPECT_EQ(-1, action); \
> + EXPECT_EQ(-(val), errno); \
> + } else { \
> + EXPECT_EQ(val, action); \
> + } \
> + } while (0)
> #endif
>
> /* Use PTRACE_GETREGS and PTRACE_SETREGS when available. This is useful for
> @@ -1602,7 +1611,7 @@ int get_syscall(struct __test_metadata *_metadata, pid_t tracee)
>
> /* Architecture-specific syscall changing routine. */
> void change_syscall(struct __test_metadata *_metadata,
> - pid_t tracee, int syscall)
> + pid_t tracee, int syscall, int result)
> {
> int ret;
> ARCH_REGS regs;
> @@ -1661,7 +1670,7 @@ void change_syscall(struct __test_metadata *_metadata,
> #ifdef SYSCALL_NUM_RET_SHARE_REG
> TH_LOG("Can't modify syscall return on this architecture");
> #else
> - regs.SYSCALL_RET = EPERM;
> + regs.SYSCALL_RET = result;
> #endif
>
> #ifdef HAVE_GETREGS
> @@ -1689,14 +1698,19 @@ void tracer_syscall(struct __test_metadata *_metadata, pid_t tracee,
> case 0x1002:
> /* change getpid to getppid. */
> EXPECT_EQ(__NR_getpid, get_syscall(_metadata, tracee));
> - change_syscall(_metadata, tracee, __NR_getppid);
> + change_syscall(_metadata, tracee, __NR_getppid, 0);
> break;
> case 0x1003:
> - /* skip gettid. */
> + /* skip gettid with valid return code. */
> EXPECT_EQ(__NR_gettid, get_syscall(_metadata, tracee));
> - change_syscall(_metadata, tracee, -1);
> + change_syscall(_metadata, tracee, -1, 45000);
> break;
> case 0x1004:
> + /* skip openat with error. */
> + EXPECT_EQ(__NR_openat, get_syscall(_metadata, tracee));
> + change_syscall(_metadata, tracee, -1, -ESRCH);
> + break;
> + case 0x1005:
> /* do nothing (allow getppid) */
> EXPECT_EQ(__NR_getppid, get_syscall(_metadata, tracee));
> break;
> @@ -1729,9 +1743,11 @@ void tracer_ptrace(struct __test_metadata *_metadata, pid_t tracee,
> nr = get_syscall(_metadata, tracee);
>
> if (nr == __NR_getpid)
> - change_syscall(_metadata, tracee, __NR_getppid);
> + change_syscall(_metadata, tracee, __NR_getppid, 0);
> + if (nr == __NR_gettid)
> + change_syscall(_metadata, tracee, -1, 45000);
> if (nr == __NR_openat)
> - change_syscall(_metadata, tracee, -1);
> + change_syscall(_metadata, tracee, -1, -ESRCH);
> }
>
> FIXTURE_DATA(TRACE_syscall) {
> @@ -1748,8 +1764,10 @@ FIXTURE_SETUP(TRACE_syscall)
> BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_TRACE | 0x1002),
> BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_gettid, 0, 1),
> BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_TRACE | 0x1003),
> - BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_getppid, 0, 1),
> + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_openat, 0, 1),
> BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_TRACE | 0x1004),
> + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_getppid, 0, 1),
> + BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_TRACE | 0x1005),
> BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
> };
>
> @@ -1797,15 +1815,26 @@ TEST_F(TRACE_syscall, ptrace_syscall_redirected)
> EXPECT_NE(self->mypid, syscall(__NR_getpid));
> }
>
> -TEST_F(TRACE_syscall, ptrace_syscall_dropped)
> +TEST_F(TRACE_syscall, ptrace_syscall_errno)
> +{
> + /* Swap SECCOMP_RET_TRACE tracer for PTRACE_SYSCALL tracer. */
> + teardown_trace_fixture(_metadata, self->tracer);
> + self->tracer = setup_trace_fixture(_metadata, tracer_ptrace, NULL,
> + true);
> +
> + /* Tracer should skip the open syscall, resulting in ESRCH. */
> + EXPECT_SYSCALL_RETURN(-ESRCH, syscall(__NR_openat));
> +}
> +
> +TEST_F(TRACE_syscall, ptrace_syscall_faked)
> {
> /* Swap SECCOMP_RET_TRACE tracer for PTRACE_SYSCALL tracer. */
> teardown_trace_fixture(_metadata, self->tracer);
> self->tracer = setup_trace_fixture(_metadata, tracer_ptrace, NULL,
> true);
>
> - /* Tracer should skip the open syscall, resulting in EPERM. */
> - EXPECT_SYSCALL_RETURN(EPERM, syscall(__NR_openat));
> + /* Tracer should skip the gettid syscall, resulting fake pid. */
> + EXPECT_SYSCALL_RETURN(45000, syscall(__NR_gettid));
> }
>
> TEST_F(TRACE_syscall, syscall_allowed)
> @@ -1838,7 +1867,21 @@ TEST_F(TRACE_syscall, syscall_redirected)
> EXPECT_NE(self->mypid, syscall(__NR_getpid));
> }
>
> -TEST_F(TRACE_syscall, syscall_dropped)
> +TEST_F(TRACE_syscall, syscall_errno)
> +{
> + long ret;
> +
> + ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
> + ASSERT_EQ(0, ret);
> +
> + ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &self->prog, 0, 0);
> + ASSERT_EQ(0, ret);
> +
> + /* openat has been skipped and an errno return. */
> + EXPECT_SYSCALL_RETURN(-ESRCH, syscall(__NR_openat));
> +}
> +
> +TEST_F(TRACE_syscall, syscall_faked)
> {
> long ret;
>
> @@ -1849,8 +1892,7 @@ TEST_F(TRACE_syscall, syscall_dropped)
> ASSERT_EQ(0, ret);
>
> /* gettid has been skipped and an altered return value stored. */
> - EXPECT_SYSCALL_RETURN(EPERM, syscall(__NR_gettid));
> - EXPECT_NE(self->mytid, syscall(__NR_gettid));
> + EXPECT_SYSCALL_RETURN(45000, syscall(__NR_gettid));
> }
>
> TEST_F(TRACE_syscall, skip_after_RET_TRACE)
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20190529/eabe56ce/attachment-0001.sig>
More information about the kernel-team
mailing list