Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752162AbbKBTta (ORCPT ); Mon, 2 Nov 2015 14:49:30 -0500 Received: from mail-ig0-f175.google.com ([209.85.213.175]:38701 "EHLO mail-ig0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751729AbbKBTt2 (ORCPT ); Mon, 2 Nov 2015 14:49:28 -0500 MIME-Version: 1.0 In-Reply-To: <1446490222-528-1-git-send-email-rsesek@google.com> References: <1446490222-528-1-git-send-email-rsesek@google.com> Date: Mon, 2 Nov 2015 11:49:27 -0800 X-Google-Sender-Auth: aqjG-ktQZMiiBZCQ9Wrjh-YcKAE Message-ID: Subject: Re: [PATCH] selftests/seccomp: Be more precise with syscall arguments. From: Kees Cook To: Shuah Khan Cc: Robert Sesek , LKML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5206 Lines: 141 On Mon, Nov 2, 2015 at 10:50 AM, Robert Sesek wrote: > Certain syscall emulation layers strictly check that the number of > arguments match what the syscall handler expects. The KILL_one_arg_one and > KILL_one_arg_six tests passed more parameters than expected to various > syscalls, causing failures in this emulation mode. Instead, test using > syscalls that take the appropriate number of arguments. > > Signed-off-by: Robert Sesek Signed-off-by: Kees Cook Looks great, thanks! Shuah, can you take this into the selftests tree? -Kees > --- > tools/testing/selftests/seccomp/seccomp_bpf.c | 46 ++++++++++++++++++++------- > 1 file changed, 35 insertions(+), 11 deletions(-) > > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c > index e7bc5d3..e38cc54 100644 > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c > @@ -29,6 +29,9 @@ > #include > #include > #include > +#include > +#include > +#include > > #define _GNU_SOURCE > #include > @@ -429,14 +432,16 @@ TEST_SIGNAL(KILL_one, SIGSYS) > > TEST_SIGNAL(KILL_one_arg_one, SIGSYS) > { > + void *fatal_address; > struct sock_filter filter[] = { > BPF_STMT(BPF_LD|BPF_W|BPF_ABS, > offsetof(struct seccomp_data, nr)), > - BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_getpid, 1, 0), > + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_times, 1, 0), > BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW), > /* Only both with lower 32-bit for now. */ > BPF_STMT(BPF_LD|BPF_W|BPF_ABS, syscall_arg(0)), > - BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x0C0FFEE, 0, 1), > + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, > + (unsigned long)&fatal_address, 0, 1), > BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_KILL), > BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW), > }; > @@ -446,7 +451,8 @@ TEST_SIGNAL(KILL_one_arg_one, SIGSYS) > }; > long ret; > pid_t parent = getppid(); > - pid_t pid = getpid(); > + struct tms timebuf; > + clock_t clock = times(&timebuf); > > ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); > ASSERT_EQ(0, ret); > @@ -455,17 +461,22 @@ TEST_SIGNAL(KILL_one_arg_one, SIGSYS) > ASSERT_EQ(0, ret); > > EXPECT_EQ(parent, syscall(__NR_getppid)); > - EXPECT_EQ(pid, syscall(__NR_getpid)); > - /* getpid() should never return. */ > - EXPECT_EQ(0, syscall(__NR_getpid, 0x0C0FFEE)); > + EXPECT_LE(clock, syscall(__NR_times, &timebuf)); > + /* times() should never return. */ > + EXPECT_EQ(0, syscall(__NR_times, &fatal_address)); > } > > TEST_SIGNAL(KILL_one_arg_six, SIGSYS) > { > +#ifndef __NR_mmap2 > + int sysno = __NR_mmap; > +#else > + int sysno = __NR_mmap2; > +#endif > struct sock_filter filter[] = { > BPF_STMT(BPF_LD|BPF_W|BPF_ABS, > offsetof(struct seccomp_data, nr)), > - BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_getpid, 1, 0), > + BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, sysno, 1, 0), > BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW), > /* Only both with lower 32-bit for now. */ > BPF_STMT(BPF_LD|BPF_W|BPF_ABS, syscall_arg(5)), > @@ -479,7 +490,8 @@ TEST_SIGNAL(KILL_one_arg_six, SIGSYS) > }; > long ret; > pid_t parent = getppid(); > - pid_t pid = getpid(); > + int fd; > + void *map1, *map2; > > ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); > ASSERT_EQ(0, ret); > @@ -487,10 +499,22 @@ TEST_SIGNAL(KILL_one_arg_six, SIGSYS) > ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog); > ASSERT_EQ(0, ret); > > + fd = open("/dev/zero", O_RDONLY); > + ASSERT_NE(-1, fd); > + > EXPECT_EQ(parent, syscall(__NR_getppid)); > - EXPECT_EQ(pid, syscall(__NR_getpid)); > - /* getpid() should never return. */ > - EXPECT_EQ(0, syscall(__NR_getpid, 1, 2, 3, 4, 5, 0x0C0FFEE)); > + map1 = (void *)syscall(sysno, > + NULL, PAGE_SIZE, PROT_READ, MAP_PRIVATE, fd, PAGE_SIZE); > + EXPECT_NE(MAP_FAILED, map1); > + /* mmap2() should never return. */ > + map2 = (void *)syscall(sysno, > + NULL, PAGE_SIZE, PROT_READ, MAP_PRIVATE, fd, 0x0C0FFEE); > + EXPECT_EQ(MAP_FAILED, map2); > + > + /* The test failed, so clean up the resources. */ > + munmap(map1, PAGE_SIZE); > + munmap(map2, PAGE_SIZE); > + close(fd); > } > > /* TODO(wad) add 64-bit versus 32-bit arg tests. */ > -- > 2.6.0.rc2.230.g3dd15c0 > -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/