| Message ID | 20190918084833.9369-3-christian.brauner@ubuntu.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | seccomp: continue syscall from notifier | expand |
On 2019-09-18 10:48:31, Christian Brauner wrote: > Add tw missing ptrace ifdefines to avoid compilation errors on systems > that do not provide PTRACE_EVENTMSG_SYSCALL_ENTRY or > PTRACE_EVENTMSG_SYSCALL_EXIT or: > > gcc -Wl,-no-as-needed -Wall seccomp_bpf.c -lpthread -o seccomp_bpf > In file included from seccomp_bpf.c:52:0: > seccomp_bpf.c: In function ‘tracer_ptrace’: > seccomp_bpf.c:1792:20: error: ‘PTRACE_EVENTMSG_SYSCALL_ENTRY’ undeclared (first use in this function); did you mean ‘PTRACE_EVENT_CLONE’? > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > ^ > ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’ > __typeof__(_expected) __exp = (_expected); \ > ^~~~~~~~~ > seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’ > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > ^~~~~~~~~ > seccomp_bpf.c:1792:20: note: each undeclared identifier is reported only once for each function it appears in > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > ^ > ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’ > __typeof__(_expected) __exp = (_expected); \ > ^~~~~~~~~ > seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’ > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > ^~~~~~~~~ > seccomp_bpf.c:1793:6: error: ‘PTRACE_EVENTMSG_SYSCALL_EXIT’ undeclared (first use in this function); did you mean ‘PTRACE_EVENTMSG_SYSCALL_ENTRY’? > : PTRACE_EVENTMSG_SYSCALL_EXIT, msg); > ^ > ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’ > __typeof__(_expected) __exp = (_expected); \ > ^~~~~~~~~ > seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’ > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > ^~~~~~~~~ > > Fixes: 6a21cc50f0c7 ("seccomp: add a return code to trap to userspace") I think this Fixes line is incorrect and should be changed to: Fixes: 201766a20e30 ("ptrace: add PTRACE_GET_SYSCALL_INFO request") With that changed, Reviewed-by: Tyler Hicks <tyhicks@canonical.com> Tyler > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > Cc: Kees Cook <keescook@chromium.org> > Cc: Andy Lutomirski <luto@amacapital.net> > Cc: Will Drewry <wad@chromium.org> > Cc: Shuah Khan <shuah@kernel.org> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > Cc: Martin KaFai Lau <kafai@fb.com> > Cc: Song Liu <songliubraving@fb.com> > Cc: Yonghong Song <yhs@fb.com> > Cc: Tycho Andersen <tycho@tycho.ws> > CC: Tyler Hicks <tyhicks@canonical.com> > Cc: Jann Horn <jannh@google.com> > Cc: stable@vger.kernel.org > Cc: linux-kselftest@vger.kernel.org > Cc: netdev@vger.kernel.org > Cc: bpf@vger.kernel.org > --- > tools/testing/selftests/seccomp/seccomp_bpf.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c > index 6ef7f16c4cf5..ee52eab01800 100644 > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c > @@ -155,6 +155,14 @@ struct seccomp_data { > #ifndef PTRACE_SECCOMP_GET_METADATA > #define PTRACE_SECCOMP_GET_METADATA 0x420d > > +#ifndef PTRACE_EVENTMSG_SYSCALL_ENTRY > +#define PTRACE_EVENTMSG_SYSCALL_ENTRY 1 > +#endif > + > +#ifndef PTRACE_EVENTMSG_SYSCALL_EXIT > +#define PTRACE_EVENTMSG_SYSCALL_EXIT 2 > +#endif > + > struct seccomp_metadata { > __u64 filter_off; /* Input: which filter */ > __u64 flags; /* Output: filter's flags */ > -- > 2.23.0 >
On Wed, Sep 18, 2019 at 11:15:12AM +0200, Tyler Hicks wrote: > On 2019-09-18 10:48:31, Christian Brauner wrote: > > Add tw missing ptrace ifdefines to avoid compilation errors on systems > > that do not provide PTRACE_EVENTMSG_SYSCALL_ENTRY or > > PTRACE_EVENTMSG_SYSCALL_EXIT or: > > > > gcc -Wl,-no-as-needed -Wall seccomp_bpf.c -lpthread -o seccomp_bpf > > In file included from seccomp_bpf.c:52:0: > > seccomp_bpf.c: In function ‘tracer_ptrace’: > > seccomp_bpf.c:1792:20: error: ‘PTRACE_EVENTMSG_SYSCALL_ENTRY’ undeclared (first use in this function); did you mean ‘PTRACE_EVENT_CLONE’? > > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > > ^ > > ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’ > > __typeof__(_expected) __exp = (_expected); \ > > ^~~~~~~~~ > > seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’ > > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > > ^~~~~~~~~ > > seccomp_bpf.c:1792:20: note: each undeclared identifier is reported only once for each function it appears in > > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > > ^ > > ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’ > > __typeof__(_expected) __exp = (_expected); \ > > ^~~~~~~~~ > > seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’ > > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > > ^~~~~~~~~ > > seccomp_bpf.c:1793:6: error: ‘PTRACE_EVENTMSG_SYSCALL_EXIT’ undeclared (first use in this function); did you mean ‘PTRACE_EVENTMSG_SYSCALL_ENTRY’? > > : PTRACE_EVENTMSG_SYSCALL_EXIT, msg); > > ^ > > ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’ > > __typeof__(_expected) __exp = (_expected); \ > > ^~~~~~~~~ > > seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’ > > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > > ^~~~~~~~~ > > > > Fixes: 6a21cc50f0c7 ("seccomp: add a return code to trap to userspace") > > I think this Fixes line is incorrect and should be changed to: > > Fixes: 201766a20e30 ("ptrace: add PTRACE_GET_SYSCALL_INFO request") > > With that changed, > > Reviewed-by: Tyler Hicks <tyhicks@canonical.com> This is actually fixed in -next already (and, yes, with the Fixes line Tyler has mentioned): https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=next&id=69b2d3c5924273a0ae968d3818210fc57a1b9d07
On Wed, Sep 18, 2019 at 10:33:09AM -0700, Kees Cook wrote: > On Wed, Sep 18, 2019 at 11:15:12AM +0200, Tyler Hicks wrote: > > On 2019-09-18 10:48:31, Christian Brauner wrote: > > > Add tw missing ptrace ifdefines to avoid compilation errors on systems > > > that do not provide PTRACE_EVENTMSG_SYSCALL_ENTRY or > > > PTRACE_EVENTMSG_SYSCALL_EXIT or: > > > > > > gcc -Wl,-no-as-needed -Wall seccomp_bpf.c -lpthread -o seccomp_bpf > > > In file included from seccomp_bpf.c:52:0: > > > seccomp_bpf.c: In function ‘tracer_ptrace’: > > > seccomp_bpf.c:1792:20: error: ‘PTRACE_EVENTMSG_SYSCALL_ENTRY’ undeclared (first use in this function); did you mean ‘PTRACE_EVENT_CLONE’? > > > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > > > ^ > > > ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’ > > > __typeof__(_expected) __exp = (_expected); \ > > > ^~~~~~~~~ > > > seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’ > > > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > > > ^~~~~~~~~ > > > seccomp_bpf.c:1792:20: note: each undeclared identifier is reported only once for each function it appears in > > > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > > > ^ > > > ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’ > > > __typeof__(_expected) __exp = (_expected); \ > > > ^~~~~~~~~ > > > seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’ > > > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > > > ^~~~~~~~~ > > > seccomp_bpf.c:1793:6: error: ‘PTRACE_EVENTMSG_SYSCALL_EXIT’ undeclared (first use in this function); did you mean ‘PTRACE_EVENTMSG_SYSCALL_ENTRY’? > > > : PTRACE_EVENTMSG_SYSCALL_EXIT, msg); > > > ^ > > > ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’ > > > __typeof__(_expected) __exp = (_expected); \ > > > ^~~~~~~~~ > > > seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’ > > > EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY > > > ^~~~~~~~~ > > > > > > Fixes: 6a21cc50f0c7 ("seccomp: add a return code to trap to userspace") > > > > I think this Fixes line is incorrect and should be changed to: > > > > Fixes: 201766a20e30 ("ptrace: add PTRACE_GET_SYSCALL_INFO request") > > > > With that changed, > > > > Reviewed-by: Tyler Hicks <tyhicks@canonical.com> > > This is actually fixed in -next already (and, yes, with the Fixes line > Tyler has mentioned): > > https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=next&id=69b2d3c5924273a0ae968d3818210fc57a1b9d07 Excuse me, does it mean that you expect each selftest to be self-hosted? I was (and still is) under impression that selftests should be built with headers installed from the tree. Is it the case, or is it not?
On Thu, Sep 19, 2019 at 01:42:51PM +0300, Dmitry V. Levin wrote: > On Wed, Sep 18, 2019 at 10:33:09AM -0700, Kees Cook wrote: > > This is actually fixed in -next already (and, yes, with the Fixes line > > Tyler has mentioned): > > > > https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=next&id=69b2d3c5924273a0ae968d3818210fc57a1b9d07 > > Excuse me, does it mean that you expect each selftest to be self-hosted? > I was (and still is) under impression that selftests should be built > with headers installed from the tree. Is it the case, or is it not? As you know (but to give others some context) there is a long-standing bug in the selftest build environment that causes these problems (it isn't including the uAPI headers) which you'd proposed to be fixed recently[1]. Did that ever get sent as a "real" patch? I don't see it in Shuah's tree; can you send it to Shuah? But even with that fixed, since the seccomp selftest has a history of being built stand-alone, I've continued to take these kinds of fixes. -Kees [1] https://lore.kernel.org/lkml/20190805094719.GA1693@altlinux.org/
On 9/19/19 10:55 AM, Kees Cook wrote: > On Thu, Sep 19, 2019 at 01:42:51PM +0300, Dmitry V. Levin wrote: >> On Wed, Sep 18, 2019 at 10:33:09AM -0700, Kees Cook wrote: >>> This is actually fixed in -next already (and, yes, with the Fixes line >>> Tyler has mentioned): >>> >>> https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=next&id=69b2d3c5924273a0ae968d3818210fc57a1b9d07 >> >> Excuse me, does it mean that you expect each selftest to be self-hosted? >> I was (and still is) under impression that selftests should be built >> with headers installed from the tree. Is it the case, or is it not? > > As you know (but to give others some context) there is a long-standing > bug in the selftest build environment that causes these problems (it > isn't including the uAPI headers) which you'd proposed to be fixed > recently[1]. Did that ever get sent as a "real" patch? I don't see it > in Shuah's tree; can you send it to Shuah? > > But even with that fixed, since the seccomp selftest has a history of > being built stand-alone, I've continued to take these kinds of fixes. > > -Kees > > [1] https://lore.kernel.org/lkml/20190805094719.GA1693@altlinux.org/ > It has been sent to kselftest list yesterday. I will pull this in for my next update. thanks, -- Shuah
On Thu, Sep 19, 2019 at 09:55:30AM -0700, Kees Cook wrote: > On Thu, Sep 19, 2019 at 01:42:51PM +0300, Dmitry V. Levin wrote: > > On Wed, Sep 18, 2019 at 10:33:09AM -0700, Kees Cook wrote: > > > This is actually fixed in -next already (and, yes, with the Fixes line > > > Tyler has mentioned): > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=next&id=69b2d3c5924273a0ae968d3818210fc57a1b9d07 > > > > Excuse me, does it mean that you expect each selftest to be self-hosted? > > I was (and still is) under impression that selftests should be built > > with headers installed from the tree. Is it the case, or is it not? > > As you know (but to give others some context) there is a long-standing > bug in the selftest build environment that causes these problems (it > isn't including the uAPI headers) which you'd proposed to be fixed > recently[1]. Did that ever get sent as a "real" patch? I don't see it > in Shuah's tree; can you send it to Shuah? > > [1] https://lore.kernel.org/lkml/20190805094719.GA1693@altlinux.org/ The [1] was an idea rather than a patch, it didn't take arch uapi headers into account. OK, I'll try to come up with a proper fix then.
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index 6ef7f16c4cf5..ee52eab01800 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -155,6 +155,14 @@ struct seccomp_data { #ifndef PTRACE_SECCOMP_GET_METADATA #define PTRACE_SECCOMP_GET_METADATA 0x420d +#ifndef PTRACE_EVENTMSG_SYSCALL_ENTRY +#define PTRACE_EVENTMSG_SYSCALL_ENTRY 1 +#endif + +#ifndef PTRACE_EVENTMSG_SYSCALL_EXIT +#define PTRACE_EVENTMSG_SYSCALL_EXIT 2 +#endif + struct seccomp_metadata { __u64 filter_off; /* Input: which filter */ __u64 flags; /* Output: filter's flags */
Add tw missing ptrace ifdefines to avoid compilation errors on systems that do not provide PTRACE_EVENTMSG_SYSCALL_ENTRY or PTRACE_EVENTMSG_SYSCALL_EXIT or: gcc -Wl,-no-as-needed -Wall seccomp_bpf.c -lpthread -o seccomp_bpf In file included from seccomp_bpf.c:52:0: seccomp_bpf.c: In function ‘tracer_ptrace’: seccomp_bpf.c:1792:20: error: ‘PTRACE_EVENTMSG_SYSCALL_ENTRY’ undeclared (first use in this function); did you mean ‘PTRACE_EVENT_CLONE’? EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY ^ ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’ __typeof__(_expected) __exp = (_expected); \ ^~~~~~~~~ seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’ EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY ^~~~~~~~~ seccomp_bpf.c:1792:20: note: each undeclared identifier is reported only once for each function it appears in EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY ^ ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’ __typeof__(_expected) __exp = (_expected); \ ^~~~~~~~~ seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’ EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY ^~~~~~~~~ seccomp_bpf.c:1793:6: error: ‘PTRACE_EVENTMSG_SYSCALL_EXIT’ undeclared (first use in this function); did you mean ‘PTRACE_EVENTMSG_SYSCALL_ENTRY’? : PTRACE_EVENTMSG_SYSCALL_EXIT, msg); ^ ../kselftest_harness.h:608:13: note: in definition of macro ‘__EXPECT’ __typeof__(_expected) __exp = (_expected); \ ^~~~~~~~~ seccomp_bpf.c:1792:2: note: in expansion of macro ‘EXPECT_EQ’ EXPECT_EQ(entry ? PTRACE_EVENTMSG_SYSCALL_ENTRY ^~~~~~~~~ Fixes: 6a21cc50f0c7 ("seccomp: add a return code to trap to userspace") Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Cc: Kees Cook <keescook@chromium.org> Cc: Andy Lutomirski <luto@amacapital.net> Cc: Will Drewry <wad@chromium.org> Cc: Shuah Khan <shuah@kernel.org> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Daniel Borkmann <daniel@iogearbox.net> Cc: Martin KaFai Lau <kafai@fb.com> Cc: Song Liu <songliubraving@fb.com> Cc: Yonghong Song <yhs@fb.com> Cc: Tycho Andersen <tycho@tycho.ws> CC: Tyler Hicks <tyhicks@canonical.com> Cc: Jann Horn <jannh@google.com> Cc: stable@vger.kernel.org Cc: linux-kselftest@vger.kernel.org Cc: netdev@vger.kernel.org Cc: bpf@vger.kernel.org --- tools/testing/selftests/seccomp/seccomp_bpf.c | 8 ++++++++ 1 file changed, 8 insertions(+)