From: Kees Cook <keescook@chromium.org> To: Michael Ellerman <mpe@ellerman.id.au> Cc: LKML <linux-kernel@vger.kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Shuah Khan <shuahkh@osg.samsung.com>, Andy Lutomirski <luto@amacapital.net>, Will Drewry <wad@chromium.org>, Andrew Morton <akpm@linux-foundation.org>, Greg KH <gregkh@linuxfoundation.org>, Mauro Carvalho Chehab <mchehab@osg.samsung.com>, "David S. Miller" <davem@davemloft.net>, Arnd Bergmann <arnd@arndb.de>, Joe Perches <joe@perches.com>, Jingoo Han <jingoohan1@gmail.com>, Linux API <linux-api@vger.kernel.org> Subject: Re: [PATCH] selftests: add seccomp suite Date: Thu, 18 Jun 2015 11:00:44 -0700 [thread overview] Message-ID: <CAGXu5jL2btPJr==+Xjd+rnPQTB2RD_Q3R7vYRUDmJpS3aeW4Hg@mail.gmail.com> (raw) In-Reply-To: <1434609685.25157.5.camel@ellerman.id.au> On Wed, Jun 17, 2015 at 11:41 PM, Michael Ellerman <mpe@ellerman.id.au> wrote: > On Wed, 2015-06-17 at 11:12 -0700, Kees Cook wrote: >> On Wed, Jun 17, 2015 at 12:31 AM, Michael Ellerman <mpe@ellerman.id.au> wrote: >> > On Tue, 2015-06-16 at 10:54 -0700, Kees Cook wrote: >> >> This imports the existing seccomp test suite into the kernel's selftests >> >> tree. It contains extensive testing of seccomp features and corner cases. >> >> There remain additional tests to move into the kernel tree, but they have >> >> not yet been ported to all the architectures seccomp supports: >> >> https://github.com/redpig/seccomp/tree/master/tests >> >> >> >> Signed-off-by: Kees Cook <keescook@chromium.org> >> >> --- >> >> MAINTAINERS | 1 + >> >> tools/testing/selftests/Makefile | 1 + >> >> tools/testing/selftests/seccomp/.gitignore | 1 + >> >> tools/testing/selftests/seccomp/Makefile | 10 + >> >> tools/testing/selftests/seccomp/seccomp_bpf.c | 2109 ++++++++++++++++++++++++ >> >> tools/testing/selftests/seccomp/test_harness.h | 537 ++++++ >> > >> > >> > Thanks very much for adding this, it would have been very helpful recently when >> > I was trying to get seccomp filter working on powerpc :) >> > >> > I get one failure in TRACE_syscall.syscall_dropped: >> > >> > seccomp_bpf.c:1394:TRACE_syscall.syscall_dropped:Expected 1 (1) == syscall(207) (18446744073709551615) >> > >> > >> > So it looks like we're returning -1 instead of 1. >> > >> > That's probably a bug in our handling of the return value, or maybe an >> > inconsistency across the arches. I'll try and find time to dig into it. >> >> Ah-ha! Excellent. Did you add an implementation for change_syscall() >> in seccomp_bpf.c? I don't have a powerpc method in there. I would have >> expected both TRACE_syscall.syscall_redirected and .syscall_dropped to >> fail without that. > > Yeah I did add a change_syscall() implementation, patch below. Great! >> If you did, maybe something isn't right with regs.SYSCALL_RET ? That's >> where the return value being tested on a skipped syscall is stored. > > Yeah I saw that too, and I think you're probably right that's where the problem > is. It doesn't seem to matter what I put in SYSCALL_RET I always get -1, so I > think there's a bug in my kernel code. > > Will try and work it out tonight. > > cheers > > > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c > index c5abe7fd7590..1bced19c54fb 100644 > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c > @@ -14,6 +14,7 @@ > #include <linux/filter.h> > #include <sys/prctl.h> > #include <sys/ptrace.h> > +#include <sys/types.h> > #include <sys/user.h> > #include <linux/prctl.h> > #include <linux/ptrace.h> > @@ -1199,6 +1200,10 @@ TEST_F(TRACE_poke, getpid_runs_normally) > # define ARCH_REGS struct user_pt_regs > # define SYSCALL_NUM regs[8] > # define SYSCALL_RET regs[0] > +#elif defined(__powerpc__) > +# define ARCH_REGS struct pt_regs > +# define SYSCALL_NUM gpr[0] > +# define SYSCALL_RET gpr[3] > #else > # error "Do not know how to find your architecture's registers and syscalls" > #endif > @@ -1246,6 +1251,10 @@ void change_syscall(struct __test_metadata *_metadata, > EXPECT_EQ(0, ret); > } > > +#elif defined(__powerpc__) > + { > + regs.SYSCALL_NUM = syscall; > + } This can be collapsed into the first #if test with the other architectures, but otherwise looks great. > #else > ASSERT_EQ(1, 0) { > TH_LOG("How is the syscall changed on this architecture?"); > @@ -1396,6 +1405,8 @@ TEST_F(TRACE_syscall, syscall_dropped) > # define __NR_seccomp 383 > # elif defined(__aarch64__) > # define __NR_seccomp 277 > +# elif defined(__powerpc__) > +# define __NR_seccomp 358 > # else > # warning "seccomp syscall number unknown for this architecture" > # define __NR_seccomp 0xffff > > > Thanks! -Kees -- Kees Cook Chrome OS Security
WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> To: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> Cc: LKML <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Daniel Borkmann <daniel-FeC+5ew28dpmcu3hnIyYJQ@public.gmane.org>, Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>, Will Drewry <wad-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>, Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>, Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>, "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>, Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>, Jingoo Han <jingoohan1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Linux API <linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> Subject: Re: [PATCH] selftests: add seccomp suite Date: Thu, 18 Jun 2015 11:00:44 -0700 [thread overview] Message-ID: <CAGXu5jL2btPJr==+Xjd+rnPQTB2RD_Q3R7vYRUDmJpS3aeW4Hg@mail.gmail.com> (raw) In-Reply-To: <1434609685.25157.5.camel-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> On Wed, Jun 17, 2015 at 11:41 PM, Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> wrote: > On Wed, 2015-06-17 at 11:12 -0700, Kees Cook wrote: >> On Wed, Jun 17, 2015 at 12:31 AM, Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> wrote: >> > On Tue, 2015-06-16 at 10:54 -0700, Kees Cook wrote: >> >> This imports the existing seccomp test suite into the kernel's selftests >> >> tree. It contains extensive testing of seccomp features and corner cases. >> >> There remain additional tests to move into the kernel tree, but they have >> >> not yet been ported to all the architectures seccomp supports: >> >> https://github.com/redpig/seccomp/tree/master/tests >> >> >> >> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> >> >> --- >> >> MAINTAINERS | 1 + >> >> tools/testing/selftests/Makefile | 1 + >> >> tools/testing/selftests/seccomp/.gitignore | 1 + >> >> tools/testing/selftests/seccomp/Makefile | 10 + >> >> tools/testing/selftests/seccomp/seccomp_bpf.c | 2109 ++++++++++++++++++++++++ >> >> tools/testing/selftests/seccomp/test_harness.h | 537 ++++++ >> > >> > >> > Thanks very much for adding this, it would have been very helpful recently when >> > I was trying to get seccomp filter working on powerpc :) >> > >> > I get one failure in TRACE_syscall.syscall_dropped: >> > >> > seccomp_bpf.c:1394:TRACE_syscall.syscall_dropped:Expected 1 (1) == syscall(207) (18446744073709551615) >> > >> > >> > So it looks like we're returning -1 instead of 1. >> > >> > That's probably a bug in our handling of the return value, or maybe an >> > inconsistency across the arches. I'll try and find time to dig into it. >> >> Ah-ha! Excellent. Did you add an implementation for change_syscall() >> in seccomp_bpf.c? I don't have a powerpc method in there. I would have >> expected both TRACE_syscall.syscall_redirected and .syscall_dropped to >> fail without that. > > Yeah I did add a change_syscall() implementation, patch below. Great! >> If you did, maybe something isn't right with regs.SYSCALL_RET ? That's >> where the return value being tested on a skipped syscall is stored. > > Yeah I saw that too, and I think you're probably right that's where the problem > is. It doesn't seem to matter what I put in SYSCALL_RET I always get -1, so I > think there's a bug in my kernel code. > > Will try and work it out tonight. > > cheers > > > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c > index c5abe7fd7590..1bced19c54fb 100644 > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c > @@ -14,6 +14,7 @@ > #include <linux/filter.h> > #include <sys/prctl.h> > #include <sys/ptrace.h> > +#include <sys/types.h> > #include <sys/user.h> > #include <linux/prctl.h> > #include <linux/ptrace.h> > @@ -1199,6 +1200,10 @@ TEST_F(TRACE_poke, getpid_runs_normally) > # define ARCH_REGS struct user_pt_regs > # define SYSCALL_NUM regs[8] > # define SYSCALL_RET regs[0] > +#elif defined(__powerpc__) > +# define ARCH_REGS struct pt_regs > +# define SYSCALL_NUM gpr[0] > +# define SYSCALL_RET gpr[3] > #else > # error "Do not know how to find your architecture's registers and syscalls" > #endif > @@ -1246,6 +1251,10 @@ void change_syscall(struct __test_metadata *_metadata, > EXPECT_EQ(0, ret); > } > > +#elif defined(__powerpc__) > + { > + regs.SYSCALL_NUM = syscall; > + } This can be collapsed into the first #if test with the other architectures, but otherwise looks great. > #else > ASSERT_EQ(1, 0) { > TH_LOG("How is the syscall changed on this architecture?"); > @@ -1396,6 +1405,8 @@ TEST_F(TRACE_syscall, syscall_dropped) > # define __NR_seccomp 383 > # elif defined(__aarch64__) > # define __NR_seccomp 277 > +# elif defined(__powerpc__) > +# define __NR_seccomp 358 > # else > # warning "seccomp syscall number unknown for this architecture" > # define __NR_seccomp 0xffff > > > Thanks! -Kees -- Kees Cook Chrome OS Security
next prev parent reply other threads:[~2015-06-18 18:00 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-06-16 17:54 [PATCH] selftests: add seccomp suite Kees Cook 2015-06-16 19:52 ` Andy Lutomirski 2015-06-16 19:52 ` Andy Lutomirski 2015-06-16 20:54 ` Daniel Borkmann 2015-06-16 20:54 ` Daniel Borkmann 2015-06-17 6:12 ` Michael Ellerman 2015-06-17 6:12 ` Michael Ellerman 2015-06-17 7:31 ` Michael Ellerman 2015-06-17 7:31 ` Michael Ellerman 2015-06-17 18:12 ` Kees Cook 2015-06-17 18:12 ` Kees Cook 2015-06-17 23:25 ` Shuah Khan 2015-06-18 6:41 ` Michael Ellerman 2015-06-18 6:41 ` Michael Ellerman 2015-06-18 18:00 ` Kees Cook [this message] 2015-06-18 18:00 ` Kees Cook
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAGXu5jL2btPJr==+Xjd+rnPQTB2RD_Q3R7vYRUDmJpS3aeW4Hg@mail.gmail.com' \ --to=keescook@chromium.org \ --cc=akpm@linux-foundation.org \ --cc=arnd@arndb.de \ --cc=daniel@iogearbox.net \ --cc=davem@davemloft.net \ --cc=gregkh@linuxfoundation.org \ --cc=jingoohan1@gmail.com \ --cc=joe@perches.com \ --cc=linux-api@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=luto@amacapital.net \ --cc=mchehab@osg.samsung.com \ --cc=mpe@ellerman.id.au \ --cc=shuahkh@osg.samsung.com \ --cc=wad@chromium.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.