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