From: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
To: Kees Cook <keescook@chromium.org>, linux-kernel@vger.kernel.org
Cc: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de>,
Rich Felker <dalias@libc.org>,
Andy Lutomirski <luto@amacapital.net>,
Will Drewry <wad@chromium.org>,
linux-kselftest@vger.kernel.org
Subject: Re: [PATCH] selftests/seccomp: sh: Fix register names
Date: Tue, 17 Nov 2020 23:57:20 +0100 [thread overview]
Message-ID: <a36d7b48-6598-1642-e403-0c77a86f416d@physik.fu-berlin.de> (raw)
In-Reply-To: <20201117205656.1000223-1-keescook@chromium.org>
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
next prev parent reply other threads:[~2020-11-17 22:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2020-11-20 19:05 ` 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=a36d7b48-6598-1642-e403-0c77a86f416d@physik.fu-berlin.de \
--to=glaubitz@physik.fu-berlin.de \
--cc=dalias@libc.org \
--cc=keescook@chromium.org \
--cc=kernel@mkarcher.dialup.fu-berlin.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=luto@amacapital.net \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).