All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] selftests/seccomp: sh: Fix register names
@ 2020-11-17 20:56 Kees Cook
  2020-11-17 21:08 ` John Paul Adrian Glaubitz
  2020-11-17 22:57 ` John Paul Adrian Glaubitz
  0 siblings, 2 replies; 6+ messages in thread
From: Kees Cook @ 2020-11-17 20:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Kees Cook, John Paul Adrian Glaubitz, Michael Karcher,
	Rich Felker, Andy Lutomirski, Will Drewry, linux-kselftest

It looks like the seccomp selftests were never actually built for sh.
This fixes it, though I don't have an environment to do a runtime test
of it yet.

Fixes: 0bb605c2c7f2b4b3 ("sh: Add SECCOMP_FILTER")
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 tools/testing/selftests/seccomp/seccomp_bpf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 7f7ecfcd66db..26c72f2b61b1 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -1804,8 +1804,8 @@ TEST_F(TRACE_poke, getpid_runs_normally)
 #define SYSCALL_RET(_regs)	(_regs).a[(_regs).windowbase * 4 + 2]
 #elif defined(__sh__)
 # define ARCH_REGS		struct pt_regs
-# define SYSCALL_NUM(_regs)	(_regs).gpr[3]
-# define SYSCALL_RET(_regs)	(_regs).gpr[0]
+# define SYSCALL_NUM(_regs)	(_regs).regs[3]
+# define SYSCALL_RET(_regs)	(_regs).regs[0]
 #else
 # error "Do not know how to find your architecture's registers and syscalls"
 #endif
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests/seccomp: sh: Fix register names
  2020-11-17 20:56 [PATCH] selftests/seccomp: sh: Fix register names Kees Cook
@ 2020-11-17 21:08 ` John Paul Adrian Glaubitz
  2020-11-17 21:35   ` Kees Cook
  2020-11-17 22:57 ` John Paul Adrian Glaubitz
  1 sibling, 1 reply; 6+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-11-17 21:08 UTC (permalink / raw)
  To: Kees Cook, linux-kernel
  Cc: Michael Karcher, Rich Felker, Andy Lutomirski, Will Drewry,
	linux-kselftest

Hi Kees!

On 11/17/20 9:56 PM, Kees Cook wrote:
> It looks like the seccomp selftests were never actually built for sh.
> This fixes it, though I don't have an environment to do a runtime test
> of it yet.

We were testing libsecomp itself but I think we might have forgotten the
self-test. Not sure how these are run.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests/seccomp: sh: Fix register names
  2020-11-17 21:08 ` John Paul Adrian Glaubitz
@ 2020-11-17 21:35   ` Kees Cook
  2020-11-17 21:41     ` John Paul Adrian Glaubitz
  0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2020-11-17 21:35 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz
  Cc: linux-kernel, Michael Karcher, Rich Felker, Andy Lutomirski,
	Will Drewry, linux-kselftest

On Tue, Nov 17, 2020 at 10:08:13PM +0100, John Paul Adrian Glaubitz wrote:
> Hi Kees!
> 
> On 11/17/20 9:56 PM, Kees Cook wrote:
> > It looks like the seccomp selftests were never actually built for sh.
> > This fixes it, though I don't have an environment to do a runtime test
> > of it yet.
> 
> We were testing libsecomp itself but I think we might have forgotten the
> self-test. Not sure how these are run.

If you're building natively, you can just build and run:

cd tools/testing/selftests/seccomp
make
./seccomp_bpf

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests/seccomp: sh: Fix register names
  2020-11-17 21:35   ` Kees Cook
@ 2020-11-17 21:41     ` John Paul Adrian Glaubitz
  0 siblings, 0 replies; 6+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-11-17 21:41 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-kernel, Michael Karcher, Rich Felker, Andy Lutomirski,
	Will Drewry, linux-kselftest

On 11/17/20 10:35 PM, Kees Cook wrote:
>> We were testing libsecomp itself but I think we might have forgotten the
>> self-test. Not sure how these are run.
> 
> If you're building natively, you can just build and run:
> 
> cd tools/testing/selftests/seccomp
> make
> ./seccomp_bpf

Thanks, I'll give that a try.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests/seccomp: sh: Fix register names
  2020-11-17 20:56 [PATCH] selftests/seccomp: sh: Fix register names Kees Cook
  2020-11-17 21:08 ` John Paul Adrian Glaubitz
@ 2020-11-17 22:57 ` John Paul Adrian Glaubitz
  2020-11-20 19:05   ` Kees Cook
  1 sibling, 1 reply; 6+ messages in thread
From: John Paul Adrian Glaubitz @ 2020-11-17 22:57 UTC (permalink / raw)
  To: Kees Cook, linux-kernel
  Cc: Michael Karcher, Rich Felker, Andy Lutomirski, Will Drewry,
	linux-kselftest

On 11/17/20 9:56 PM, Kees Cook wrote:
> It looks like the seccomp selftests were never actually built for sh.
> This fixes it, though I don't have an environment to do a runtime test
> of it yet.
> 
> Fixes: 0bb605c2c7f2b4b3 ("sh: Add SECCOMP_FILTER")
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  tools/testing/selftests/seccomp/seccomp_bpf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index 7f7ecfcd66db..26c72f2b61b1 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -1804,8 +1804,8 @@ TEST_F(TRACE_poke, getpid_runs_normally)
>  #define SYSCALL_RET(_regs)	(_regs).a[(_regs).windowbase * 4 + 2]
>  #elif defined(__sh__)
>  # define ARCH_REGS		struct pt_regs
> -# define SYSCALL_NUM(_regs)	(_regs).gpr[3]
> -# define SYSCALL_RET(_regs)	(_regs).gpr[0]
> +# define SYSCALL_NUM(_regs)	(_regs).regs[3]
> +# define SYSCALL_RET(_regs)	(_regs).regs[0]
>  #else
>  # error "Do not know how to find your architecture's registers and syscalls"
>  #endif

Yes, this fix is indeed necessary. However, there is another build issue that I ran into
and I'm not sure why it happens, but commenting out "#include <linux/sched.h>" in
../clone3/clone3_selftests.h fixes it.

root@tirpitz:..selftests/seccomp> make
gcc -Wl,-no-as-needed -Wall  -lpthread  seccomp_bpf.c /usr/src/linux-5.9.8/tools/testing/selftests/kselftest_harness.h /usr/src/linux-5.9.8/tools/testing/selftests/kselftest.h  -o /usr/src/linux-5.9.8/tools/testing/selftests/seccomp/seccomp_bpf
In file included from seccomp_bpf.c:55:
../clone3/clone3_selftests.h:28:8: error: redefinition of ‘struct clone_args’
   28 | struct clone_args {
      |        ^~~~~~~~~~
In file included from ../clone3/clone3_selftests.h:8,
                 from seccomp_bpf.c:55:
/usr/include/linux/sched.h:92:8: note: originally defined here
   92 | struct clone_args {
      |        ^~~~~~~~~~
make: *** [../lib.mk:140: /usr/src/linux-5.9.8/tools/testing/selftests/seccomp/seccomp_bpf] Error 1
root@tirpitz:..selftests/seccomp>

Your actual register naming fix is correct in any case as without your patch, building the seccomp
selftest fails with:

seccomp_bpf.c: In function ‘get_syscall’:
seccomp_bpf.c:1741:37: error: ‘struct pt_regs’ has no member named ‘gpr’; did you mean ‘pr’?
 1741 | # define SYSCALL_NUM(_regs) (_regs).gpr[3]
      |                                     ^~~
seccomp_bpf.c:1794:9: note: in expansion of macro ‘SYSCALL_NUM’
 1794 |  return SYSCALL_NUM(regs);
      |         ^~~~~~~~~~~
seccomp_bpf.c: In function ‘change_syscall’:
seccomp_bpf.c:1741:37: error: ‘struct pt_regs’ has no member named ‘gpr’; did you mean ‘pr’?
 1741 | # define SYSCALL_NUM(_regs) (_regs).gpr[3]
      |                                     ^~~
seccomp_bpf.c:1817:3: note: in expansion of macro ‘SYSCALL_NUM’
 1817 |   SYSCALL_NUM(regs) = syscall;
      |   ^~~~~~~~~~~
seccomp_bpf.c:1742:37: error: ‘struct pt_regs’ has no member named ‘gpr’; did you mean ‘pr’?
 1742 | # define SYSCALL_RET(_regs) (_regs).gpr[0]
      |                                     ^~~
seccomp_bpf.c:1859:3: note: in expansion of macro ‘SYSCALL_RET’
 1859 |   SYSCALL_RET(regs) = result;
      |   ^~~~~~~~~~~
seccomp_bpf.c: In function ‘get_syscall’:
seccomp_bpf.c:1795:1: warning: control reaches end of non-void function [-Wreturn-type]
 1795 | }
      | ^
make: *** [../lib.mk:140: /usr/src/linux-5.9.8/tools/testing/selftests/seccomp/seccomp_bpf] Error 1

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] selftests/seccomp: sh: Fix register names
  2020-11-17 22:57 ` John Paul Adrian Glaubitz
@ 2020-11-20 19:05   ` Kees Cook
  0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2020-11-20 19:05 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz
  Cc: linux-kernel, Michael Karcher, Rich Felker, Andy Lutomirski,
	Will Drewry, linux-kselftest

On Tue, Nov 17, 2020 at 11:57:20PM +0100, John Paul Adrian Glaubitz wrote:
> On 11/17/20 9:56 PM, Kees Cook wrote:
> > It looks like the seccomp selftests were never actually built for sh.
> > This fixes it, though I don't have an environment to do a runtime test
> > of it yet.
> > 
> > Fixes: 0bb605c2c7f2b4b3 ("sh: Add SECCOMP_FILTER")
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  tools/testing/selftests/seccomp/seccomp_bpf.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> > index 7f7ecfcd66db..26c72f2b61b1 100644
> > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> > @@ -1804,8 +1804,8 @@ TEST_F(TRACE_poke, getpid_runs_normally)
> >  #define SYSCALL_RET(_regs)	(_regs).a[(_regs).windowbase * 4 + 2]
> >  #elif defined(__sh__)
> >  # define ARCH_REGS		struct pt_regs
> > -# define SYSCALL_NUM(_regs)	(_regs).gpr[3]
> > -# define SYSCALL_RET(_regs)	(_regs).gpr[0]
> > +# define SYSCALL_NUM(_regs)	(_regs).regs[3]
> > +# define SYSCALL_RET(_regs)	(_regs).regs[0]
> >  #else
> >  # error "Do not know how to find your architecture's registers and syscalls"
> >  #endif
> 
> Yes, this fix is indeed necessary. However, there is another build issue that I ran into

Okay, thanks! (I'll add a Tested-by for you...)

> and I'm not sure why it happens, but commenting out "#include <linux/sched.h>" in
> ../clone3/clone3_selftests.h fixes it.
> 
> root@tirpitz:..selftests/seccomp> make
> gcc -Wl,-no-as-needed -Wall  -lpthread  seccomp_bpf.c /usr/src/linux-5.9.8/tools/testing/selftests/kselftest_harness.h /usr/src/linux-5.9.8/tools/testing/selftests/kselftest.h  -o /usr/src/linux-5.9.8/tools/testing/selftests/seccomp/seccomp_bpf
> In file included from seccomp_bpf.c:55:
> ../clone3/clone3_selftests.h:28:8: error: redefinition of ‘struct clone_args’
>    28 | struct clone_args {
>       |        ^~~~~~~~~~
> In file included from ../clone3/clone3_selftests.h:8,
>                  from seccomp_bpf.c:55:
> /usr/include/linux/sched.h:92:8: note: originally defined here
>    92 | struct clone_args {
>       |        ^~~~~~~~~~
> make: *** [../lib.mk:140: /usr/src/linux-5.9.8/tools/testing/selftests/seccomp/seccomp_bpf] Error 1

I think this was fixed separately:
https://lore.kernel.org/lkml/160464840151.97255.15344214064240834294.sendpatchset@MacBook-Pro.local/

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-20 19:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-17 20:56 [PATCH] selftests/seccomp: sh: Fix register names Kees Cook
2020-11-17 21:08 ` John Paul Adrian Glaubitz
2020-11-17 21:35   ` Kees Cook
2020-11-17 21:41     ` John Paul Adrian Glaubitz
2020-11-17 22:57 ` John Paul Adrian Glaubitz
2020-11-20 19:05   ` Kees Cook

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.