2019-09-18 09:36:15

by Christian Brauner

[permalink] [raw]
Subject: [PATCH 2/4] seccomp: add two missing ptrace ifdefines

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 <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Will Drewry <[email protected]>
Cc: Shuah Khan <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
Cc: Daniel Borkmann <[email protected]>
Cc: Martin KaFai Lau <[email protected]>
Cc: Song Liu <[email protected]>
Cc: Yonghong Song <[email protected]>
Cc: Tycho Andersen <[email protected]>
CC: Tyler Hicks <[email protected]>
Cc: Jann Horn <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
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


2019-09-18 15:05:52

by Tyler Hicks

[permalink] [raw]
Subject: Re: [PATCH 2/4] seccomp: add two missing ptrace ifdefines

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 <[email protected]>

Tyler

> Signed-off-by: Christian Brauner <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Will Drewry <[email protected]>
> Cc: Shuah Khan <[email protected]>
> Cc: Alexei Starovoitov <[email protected]>
> Cc: Daniel Borkmann <[email protected]>
> Cc: Martin KaFai Lau <[email protected]>
> Cc: Song Liu <[email protected]>
> Cc: Yonghong Song <[email protected]>
> Cc: Tycho Andersen <[email protected]>
> CC: Tyler Hicks <[email protected]>
> Cc: Jann Horn <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> 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
>

2019-09-18 17:51:33

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 2/4] seccomp: add two missing ptrace ifdefines

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 <[email protected]>

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

--
Kees Cook

2019-09-19 12:36:24

by Dmitry V. Levin

[permalink] [raw]
Subject: Re: [PATCH 2/4] seccomp: add two missing ptrace ifdefines

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 <[email protected]>
>
> 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?


--
ldv

2019-09-19 18:44:32

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 2/4] seccomp: add two missing ptrace ifdefines

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/[email protected]/

--
Kees Cook

2019-09-20 01:25:43

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH 2/4] seccomp: add two missing ptrace ifdefines

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/[email protected]/
>

It has been sent to kselftest list yesterday. I will pull this in for
my next update.

thanks,
-- Shuah

2019-09-20 03:03:35

by Dmitry V. Levin

[permalink] [raw]
Subject: Re: [PATCH 2/4] seccomp: add two missing ptrace ifdefines

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/[email protected]/

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.


--
ldv