From: Michael Ellerman <mpe@ellerman.id.au>
To: Kees Cook <keescook@chromium.org>
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 16:41:25 +1000 [thread overview]
Message-ID: <1434609685.25157.5.camel@ellerman.id.au> (raw)
In-Reply-To: <CAGXu5jK3xuCqJoE9W+b_5yH+TffDDaL5tDFyZhKRx_K-Qqqk2g@mail.gmail.com>
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.
> 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;
+ }
#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
WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
To: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@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 16:41:25 +1000 [thread overview]
Message-ID: <1434609685.25157.5.camel@ellerman.id.au> (raw)
In-Reply-To: <CAGXu5jK3xuCqJoE9W+b_5yH+TffDDaL5tDFyZhKRx_K-Qqqk2g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
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.
> 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;
+ }
#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
next prev parent reply other threads:[~2015-06-18 6:41 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 [this message]
2015-06-18 6:41 ` Michael Ellerman
2015-06-18 18:00 ` Kees Cook
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=1434609685.25157.5.camel@ellerman.id.au \
--to=mpe@ellerman.id.au \
--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=keescook@chromium.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mchehab@osg.samsung.com \
--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: link
Be 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.