* [PATCH 1/4] sh: Fix validation of system call number
@ 2020-07-22 23:13 Michael Karcher
2020-07-22 23:13 ` [PATCH 2/4] sh: Rearrange blocks in entry-common.S Michael Karcher
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Michael Karcher @ 2020-07-22 23:13 UTC (permalink / raw)
To: linux-sh, linux-kernel
Cc: Yoshinori Sato, Rich Felker, Adrian Glaubitz, Michael Karcher
The slow path for traced system call entries accessed a wrong memory
location to get the number of the maximum allowed system call number.
Renumber the numbered "local" label for the correct location to avoid
collisions with actual local labels.
Signed-off-by: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de>
---
arch/sh/kernel/entry-common.S | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/sh/kernel/entry-common.S b/arch/sh/kernel/entry-common.S
index 956a7a03b0c8..9bac5bbb67f3 100644
--- a/arch/sh/kernel/entry-common.S
+++ b/arch/sh/kernel/entry-common.S
@@ -199,7 +199,7 @@ syscall_trace_entry:
mov.l @(OFF_R7,r15), r7 ! arg3
mov.l @(OFF_R3,r15), r3 ! syscall_nr
!
- mov.l 2f, r10 ! Number of syscalls
+ mov.l 6f, r10 ! Number of syscalls
cmp/hs r10, r3
bf syscall_call
mov #-ENOSYS, r0
@@ -353,7 +353,7 @@ ENTRY(system_call)
tst r9, r8
bf syscall_trace_entry
!
- mov.l 2f, r8 ! Number of syscalls
+ mov.l 6f, r8 ! Number of syscalls
cmp/hs r8, r3
bt syscall_badsys
!
@@ -392,7 +392,7 @@ syscall_exit:
#if !defined(CONFIG_CPU_SH2)
1: .long TRA
#endif
-2: .long NR_syscalls
+6: .long NR_syscalls
3: .long sys_call_table
7: .long do_syscall_trace_enter
8: .long do_syscall_trace_leave
--
2.28.0.rc1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 2/4] sh: Rearrange blocks in entry-common.S 2020-07-22 23:13 [PATCH 1/4] sh: Fix validation of system call number Michael Karcher @ 2020-07-22 23:13 ` Michael Karcher 2020-07-22 23:20 ` John Paul Adrian Glaubitz 2020-07-22 23:13 ` [PATCH 3/4] sh: Add SECCOMP_FILTER Michael Karcher ` (2 subsequent siblings) 3 siblings, 1 reply; 20+ messages in thread From: Michael Karcher @ 2020-07-22 23:13 UTC (permalink / raw) To: linux-sh, linux-kernel Cc: Yoshinori Sato, Rich Felker, Adrian Glaubitz, Michael Karcher This avoids out-of-range jumps that get auto-replaced by the assembler and prepares for the changes needed to implement SECCOMP_FILTER cleanly. Signed-off-by: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de> --- arch/sh/kernel/entry-common.S | 57 ++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/arch/sh/kernel/entry-common.S b/arch/sh/kernel/entry-common.S index 9bac5bbb67f3..c4d88d61890d 100644 --- a/arch/sh/kernel/entry-common.S +++ b/arch/sh/kernel/entry-common.S @@ -178,34 +178,6 @@ syscall_exit_work: bra resume_userspace nop - .align 2 -syscall_trace_entry: - ! Yes it is traced. - mov r15, r4 - mov.l 7f, r11 ! Call do_syscall_trace_enter which notifies - jsr @r11 ! superior (will chomp R[0-7]) - nop - mov.l r0, @(OFF_R0,r15) ! Save return value - ! Reload R0-R4 from kernel stack, where the - ! parent may have modified them using - ! ptrace(POKEUSR). (Note that R0-R2 are - ! reloaded from the kernel stack by syscall_call - ! below, so don't need to be reloaded here.) - ! This allows the parent to rewrite system calls - ! and args on the fly. - mov.l @(OFF_R4,r15), r4 ! arg0 - mov.l @(OFF_R5,r15), r5 - mov.l @(OFF_R6,r15), r6 - mov.l @(OFF_R7,r15), r7 ! arg3 - mov.l @(OFF_R3,r15), r3 ! syscall_nr - ! - mov.l 6f, r10 ! Number of syscalls - cmp/hs r10, r3 - bf syscall_call - mov #-ENOSYS, r0 - bra syscall_exit - mov.l r0, @(OFF_R0,r15) ! Return value - __restore_all: mov #OFF_SR, r0 mov.l @(r0,r15), r0 ! get status register @@ -388,6 +360,35 @@ syscall_exit: bf syscall_exit_work bra __restore_all nop + + .align 2 +syscall_trace_entry: + ! Yes it is traced. + mov r15, r4 + mov.l 7f, r11 ! Call do_syscall_trace_enter which notifies + jsr @r11 ! superior (will chomp R[0-7]) + nop + mov.l r0, @(OFF_R0,r15) ! Save return value + ! Reload R0-R4 from kernel stack, where the + ! parent may have modified them using + ! ptrace(POKEUSR). (Note that R0-R2 are + ! reloaded from the kernel stack by syscall_call + ! below, so don't need to be reloaded here.) + ! This allows the parent to rewrite system calls + ! and args on the fly. + mov.l @(OFF_R4,r15), r4 ! arg0 + mov.l @(OFF_R5,r15), r5 + mov.l @(OFF_R6,r15), r6 + mov.l @(OFF_R7,r15), r7 ! arg3 + mov.l @(OFF_R3,r15), r3 ! syscall_nr + ! + mov.l 6f, r10 ! Number of syscalls + cmp/hs r10, r3 + bf syscall_call + mov #-ENOSYS, r0 + bra syscall_exit + mov.l r0, @(OFF_R0,r15) ! Return value + .align 2 #if !defined(CONFIG_CPU_SH2) 1: .long TRA -- 2.28.0.rc1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] sh: Rearrange blocks in entry-common.S 2020-07-22 23:13 ` [PATCH 2/4] sh: Rearrange blocks in entry-common.S Michael Karcher @ 2020-07-22 23:20 ` John Paul Adrian Glaubitz 0 siblings, 0 replies; 20+ messages in thread From: John Paul Adrian Glaubitz @ 2020-07-22 23:20 UTC (permalink / raw) To: Michael Karcher, linux-sh, linux-kernel; +Cc: Yoshinori Sato, Rich Felker On 7/23/20 1:13 AM, Michael Karcher wrote: > This avoids out-of-range jumps that get auto-replaced by the assembler > and prepares for the changes needed to implement SECCOMP_FILTER cleanly. > > Signed-off-by: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de> > --- > arch/sh/kernel/entry-common.S | 57 ++++++++++++++++++----------------- > 1 file changed, 29 insertions(+), 28 deletions(-) > > diff --git a/arch/sh/kernel/entry-common.S b/arch/sh/kernel/entry-common.S > index 9bac5bbb67f3..c4d88d61890d 100644 > --- a/arch/sh/kernel/entry-common.S > +++ b/arch/sh/kernel/entry-common.S > @@ -178,34 +178,6 @@ syscall_exit_work: > bra resume_userspace > nop > > - .align 2 > -syscall_trace_entry: > - ! Yes it is traced. > - mov r15, r4 > - mov.l 7f, r11 ! Call do_syscall_trace_enter which notifies > - jsr @r11 ! superior (will chomp R[0-7]) > - nop > - mov.l r0, @(OFF_R0,r15) ! Save return value > - ! Reload R0-R4 from kernel stack, where the > - ! parent may have modified them using > - ! ptrace(POKEUSR). (Note that R0-R2 are > - ! reloaded from the kernel stack by syscall_call > - ! below, so don't need to be reloaded here.) > - ! This allows the parent to rewrite system calls > - ! and args on the fly. > - mov.l @(OFF_R4,r15), r4 ! arg0 > - mov.l @(OFF_R5,r15), r5 > - mov.l @(OFF_R6,r15), r6 > - mov.l @(OFF_R7,r15), r7 ! arg3 > - mov.l @(OFF_R3,r15), r3 ! syscall_nr > - ! > - mov.l 6f, r10 ! Number of syscalls > - cmp/hs r10, r3 > - bf syscall_call > - mov #-ENOSYS, r0 > - bra syscall_exit > - mov.l r0, @(OFF_R0,r15) ! Return value > - > __restore_all: > mov #OFF_SR, r0 > mov.l @(r0,r15), r0 ! get status register > @@ -388,6 +360,35 @@ syscall_exit: > bf syscall_exit_work > bra __restore_all > nop > + > + .align 2 > +syscall_trace_entry: > + ! Yes it is traced. > + mov r15, r4 > + mov.l 7f, r11 ! Call do_syscall_trace_enter which notifies > + jsr @r11 ! superior (will chomp R[0-7]) > + nop > + mov.l r0, @(OFF_R0,r15) ! Save return value > + ! Reload R0-R4 from kernel stack, where the > + ! parent may have modified them using > + ! ptrace(POKEUSR). (Note that R0-R2 are > + ! reloaded from the kernel stack by syscall_call > + ! below, so don't need to be reloaded here.) > + ! This allows the parent to rewrite system calls > + ! and args on the fly. > + mov.l @(OFF_R4,r15), r4 ! arg0 > + mov.l @(OFF_R5,r15), r5 > + mov.l @(OFF_R6,r15), r6 > + mov.l @(OFF_R7,r15), r7 ! arg3 > + mov.l @(OFF_R3,r15), r3 ! syscall_nr > + ! > + mov.l 6f, r10 ! Number of syscalls > + cmp/hs r10, r3 > + bf syscall_call > + mov #-ENOSYS, r0 > + bra syscall_exit > + mov.l r0, @(OFF_R0,r15) ! Return value > + > .align 2 > #if !defined(CONFIG_CPU_SH2) > 1: .long TRA > Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> -- .''`. 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] 20+ messages in thread
* [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-07-22 23:13 [PATCH 1/4] sh: Fix validation of system call number Michael Karcher 2020-07-22 23:13 ` [PATCH 2/4] sh: Rearrange blocks in entry-common.S Michael Karcher @ 2020-07-22 23:13 ` Michael Karcher 2020-07-22 23:20 ` John Paul Adrian Glaubitz 2020-08-28 15:50 ` Rich Felker 2020-07-22 23:13 ` [PATCH 4/4] sh: bring syscall_set_return_value in line with other architectures Michael Karcher 2020-07-22 23:19 ` [PATCH 1/4] sh: Fix validation of system call number John Paul Adrian Glaubitz 3 siblings, 2 replies; 20+ messages in thread From: Michael Karcher @ 2020-07-22 23:13 UTC (permalink / raw) To: linux-sh, linux-kernel Cc: Yoshinori Sato, Rich Felker, Adrian Glaubitz, Michael Karcher Port sh to use the new SECCOMP_FILTER code. Signed-off-by: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de> --- arch/sh/Kconfig | 1 + arch/sh/kernel/entry-common.S | 2 ++ arch/sh/kernel/ptrace_32.c | 5 +++-- tools/testing/selftests/seccomp/seccomp_bpf.c | 8 +++++++- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig index 32d959849df9..10b510c16841 100644 --- a/arch/sh/Kconfig +++ b/arch/sh/Kconfig @@ -27,6 +27,7 @@ config SUPERH select GENERIC_SMP_IDLE_THREAD select GUP_GET_PTE_LOW_HIGH if X2TLB select HAVE_ARCH_AUDITSYSCALL + select HAVE_ARCH_SECCOMP_FILTER select HAVE_ARCH_KGDB select HAVE_ARCH_TRACEHOOK select HAVE_DEBUG_BUGVERBOSE diff --git a/arch/sh/kernel/entry-common.S b/arch/sh/kernel/entry-common.S index c4d88d61890d..ad963104d22d 100644 --- a/arch/sh/kernel/entry-common.S +++ b/arch/sh/kernel/entry-common.S @@ -368,6 +368,8 @@ syscall_trace_entry: mov.l 7f, r11 ! Call do_syscall_trace_enter which notifies jsr @r11 ! superior (will chomp R[0-7]) nop + cmp/eq #-1, r0 + bt syscall_exit mov.l r0, @(OFF_R0,r15) ! Save return value ! Reload R0-R4 from kernel stack, where the ! parent may have modified them using diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c index 64bfb714943e..25ccfbd02bfa 100644 --- a/arch/sh/kernel/ptrace_32.c +++ b/arch/sh/kernel/ptrace_32.c @@ -485,8 +485,6 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) { long ret = 0; - secure_computing_strict(regs->regs[0]); - if (test_thread_flag(TIF_SYSCALL_TRACE) && tracehook_report_syscall_entry(regs)) /* @@ -496,6 +494,9 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) */ ret = -1L; + if (secure_computing() = -1) + return -1; + if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) trace_sys_enter(regs, regs->regs[0]); diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index 252140a52553..6eb21685c88f 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -122,6 +122,8 @@ struct seccomp_data { # define __NR_seccomp 358 # elif defined(__s390__) # define __NR_seccomp 348 +# elif defined(__sh__) +# define __NR_seccomp 372 # else # warning "seccomp syscall number unknown for this architecture" # define __NR_seccomp 0xffff @@ -1622,6 +1624,10 @@ TEST_F(TRACE_poke, getpid_runs_normally) # define SYSCALL_SYSCALL_NUM regs[4] # define SYSCALL_RET regs[2] # define SYSCALL_NUM_RET_SHARE_REG +#elif defined(__sh__) +# define ARCH_REGS struct pt_regs +# define SYSCALL_NUM gpr[3] +# define SYSCALL_RET gpr[0] #else # error "Do not know how to find your architecture's registers and syscalls" #endif @@ -1693,7 +1699,7 @@ void change_syscall(struct __test_metadata *_metadata, EXPECT_EQ(0, ret) {} #if defined(__x86_64__) || defined(__i386__) || defined(__powerpc__) || \ - defined(__s390__) || defined(__hppa__) || defined(__riscv) + defined(__s390__) || defined(__hppa__) || defined(__riscv) || defined(__sh__) { regs.SYSCALL_NUM = syscall; } -- 2.28.0.rc1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-07-22 23:13 ` [PATCH 3/4] sh: Add SECCOMP_FILTER Michael Karcher @ 2020-07-22 23:20 ` John Paul Adrian Glaubitz 2020-08-28 15:50 ` Rich Felker 1 sibling, 0 replies; 20+ messages in thread From: John Paul Adrian Glaubitz @ 2020-07-22 23:20 UTC (permalink / raw) To: Michael Karcher, linux-sh, linux-kernel; +Cc: Yoshinori Sato, Rich Felker On 7/23/20 1:13 AM, Michael Karcher wrote: > Port sh to use the new SECCOMP_FILTER code. > > Signed-off-by: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de> > --- > arch/sh/Kconfig | 1 + > arch/sh/kernel/entry-common.S | 2 ++ > arch/sh/kernel/ptrace_32.c | 5 +++-- > tools/testing/selftests/seccomp/seccomp_bpf.c | 8 +++++++- > 4 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig > index 32d959849df9..10b510c16841 100644 > --- a/arch/sh/Kconfig > +++ b/arch/sh/Kconfig > @@ -27,6 +27,7 @@ config SUPERH > select GENERIC_SMP_IDLE_THREAD > select GUP_GET_PTE_LOW_HIGH if X2TLB > select HAVE_ARCH_AUDITSYSCALL > + select HAVE_ARCH_SECCOMP_FILTER > select HAVE_ARCH_KGDB > select HAVE_ARCH_TRACEHOOK > select HAVE_DEBUG_BUGVERBOSE > diff --git a/arch/sh/kernel/entry-common.S b/arch/sh/kernel/entry-common.S > index c4d88d61890d..ad963104d22d 100644 > --- a/arch/sh/kernel/entry-common.S > +++ b/arch/sh/kernel/entry-common.S > @@ -368,6 +368,8 @@ syscall_trace_entry: > mov.l 7f, r11 ! Call do_syscall_trace_enter which notifies > jsr @r11 ! superior (will chomp R[0-7]) > nop > + cmp/eq #-1, r0 > + bt syscall_exit > mov.l r0, @(OFF_R0,r15) ! Save return value > ! Reload R0-R4 from kernel stack, where the > ! parent may have modified them using > diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c > index 64bfb714943e..25ccfbd02bfa 100644 > --- a/arch/sh/kernel/ptrace_32.c > +++ b/arch/sh/kernel/ptrace_32.c > @@ -485,8 +485,6 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > { > long ret = 0; > > - secure_computing_strict(regs->regs[0]); > - > if (test_thread_flag(TIF_SYSCALL_TRACE) && > tracehook_report_syscall_entry(regs)) > /* > @@ -496,6 +494,9 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > */ > ret = -1L; > > + if (secure_computing() = -1) > + return -1; > + > if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) > trace_sys_enter(regs, regs->regs[0]); > > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c > index 252140a52553..6eb21685c88f 100644 > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c > @@ -122,6 +122,8 @@ struct seccomp_data { > # define __NR_seccomp 358 > # elif defined(__s390__) > # define __NR_seccomp 348 > +# elif defined(__sh__) > +# define __NR_seccomp 372 > # else > # warning "seccomp syscall number unknown for this architecture" > # define __NR_seccomp 0xffff > @@ -1622,6 +1624,10 @@ TEST_F(TRACE_poke, getpid_runs_normally) > # define SYSCALL_SYSCALL_NUM regs[4] > # define SYSCALL_RET regs[2] > # define SYSCALL_NUM_RET_SHARE_REG > +#elif defined(__sh__) > +# define ARCH_REGS struct pt_regs > +# define SYSCALL_NUM gpr[3] > +# define SYSCALL_RET gpr[0] > #else > # error "Do not know how to find your architecture's registers and syscalls" > #endif > @@ -1693,7 +1699,7 @@ void change_syscall(struct __test_metadata *_metadata, > EXPECT_EQ(0, ret) {} > > #if defined(__x86_64__) || defined(__i386__) || defined(__powerpc__) || \ > - defined(__s390__) || defined(__hppa__) || defined(__riscv) > + defined(__s390__) || defined(__hppa__) || defined(__riscv) || defined(__sh__) > { > regs.SYSCALL_NUM = syscall; > } > Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> -- .''`. 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] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-07-22 23:13 ` [PATCH 3/4] sh: Add SECCOMP_FILTER Michael Karcher 2020-07-22 23:20 ` John Paul Adrian Glaubitz @ 2020-08-28 15:50 ` Rich Felker 2020-08-28 16:21 ` John Paul Adrian Glaubitz 2020-08-28 16:30 ` Rich Felker 1 sibling, 2 replies; 20+ messages in thread From: Rich Felker @ 2020-08-28 15:50 UTC (permalink / raw) To: Michael Karcher; +Cc: linux-sh, linux-kernel, Yoshinori Sato, Adrian Glaubitz On Thu, Jul 23, 2020 at 01:13:21AM +0200, Michael Karcher wrote: > Port sh to use the new SECCOMP_FILTER code. > > Signed-off-by: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de> > --- > arch/sh/Kconfig | 1 + > arch/sh/kernel/entry-common.S | 2 ++ > arch/sh/kernel/ptrace_32.c | 5 +++-- > tools/testing/selftests/seccomp/seccomp_bpf.c | 8 +++++++- > 4 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig > index 32d959849df9..10b510c16841 100644 > --- a/arch/sh/Kconfig > +++ b/arch/sh/Kconfig > @@ -27,6 +27,7 @@ config SUPERH > select GENERIC_SMP_IDLE_THREAD > select GUP_GET_PTE_LOW_HIGH if X2TLB > select HAVE_ARCH_AUDITSYSCALL > + select HAVE_ARCH_SECCOMP_FILTER > select HAVE_ARCH_KGDB > select HAVE_ARCH_TRACEHOOK > select HAVE_DEBUG_BUGVERBOSE > diff --git a/arch/sh/kernel/entry-common.S b/arch/sh/kernel/entry-common.S > index c4d88d61890d..ad963104d22d 100644 > --- a/arch/sh/kernel/entry-common.S > +++ b/arch/sh/kernel/entry-common.S > @@ -368,6 +368,8 @@ syscall_trace_entry: > mov.l 7f, r11 ! Call do_syscall_trace_enter which notifies > jsr @r11 ! superior (will chomp R[0-7]) > nop > + cmp/eq #-1, r0 > + bt syscall_exit > mov.l r0, @(OFF_R0,r15) ! Save return value > ! Reload R0-R4 from kernel stack, where the > ! parent may have modified them using > diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c > index 64bfb714943e..25ccfbd02bfa 100644 > --- a/arch/sh/kernel/ptrace_32.c > +++ b/arch/sh/kernel/ptrace_32.c > @@ -485,8 +485,6 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > { > long ret = 0; > > - secure_computing_strict(regs->regs[0]); > - > if (test_thread_flag(TIF_SYSCALL_TRACE) && > tracehook_report_syscall_entry(regs)) > /* > @@ -496,6 +494,9 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > */ > ret = -1L; > > + if (secure_computing() = -1) > + return -1; > + > if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) > trace_sys_enter(regs, regs->regs[0]); > This patch broke strace - it spews out bogus syscalls and gets the tracee hung. I suspect the last hunk is wrong and breaks all non-seccomp tracing. I'll follow up with further analysis and possibly a fix if you don't find one sooner. Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-08-28 15:50 ` Rich Felker @ 2020-08-28 16:21 ` John Paul Adrian Glaubitz 2020-08-28 16:30 ` Rich Felker 1 sibling, 0 replies; 20+ messages in thread From: John Paul Adrian Glaubitz @ 2020-08-28 16:21 UTC (permalink / raw) To: Rich Felker, Michael Karcher; +Cc: linux-sh, linux-kernel, Yoshinori Sato On 8/28/20 5:50 PM, Rich Felker wrote: > This patch broke strace - it spews out bogus syscalls and gets the > tracee hung. I suspect the last hunk is wrong and breaks all > non-seccomp tracing. I'll follow up with further analysis and possibly > a fix if you don't find one sooner. Hmm, it does not hang for me but it looks suspicious: root@tirpitz:~> strace ./test execve("./test", ["./test"], 0x7be59690 /* 24 vars */) = 0 brk(NULL) = 0x52abc000 uname({sysname="Linux", nodename="tirpitz", ...}) = 0 access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or directory) openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_sizeg578, ...}) = 0 mmap2(NULL, 67578, PROT_READ, MAP_PRIVATE, 3, 0) = 0x29584000 close(3) = 0 mmap2(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = -1 EPERM (Operation not permitted) openat(AT_FDCWD, "/lib/sh4-linux-gnu/tls/sh4a/libc.so.6", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = -1 ENOENT (No such file or directory) stat64("/lib/sh4-linux-gnu/tls/sh4a", 0x7b89fb2c) = -1 ENOENT (No such file or directory) openat(AT_FDCWD, "/lib/sh4-linux-gnu/tls/libc.so.6", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = -1 ENOENT (No such file or directory) stat64("/lib/sh4-linux-gnu/tls", 0x7b89fb2c) = -1 ENOENT (No such file or directory) openat(AT_FDCWD, "/lib/sh4-linux-gnu/sh4a/libc.so.6", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = -1 ENOENT (No such file or directory) stat64("/lib/sh4-linux-gnu/sh4a", 0x7b89fb2c) = -1 ENOENT (No such file or directory) openat(AT_FDCWD, "/lib/sh4-linux-gnu/libc.so.6", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3 read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0*\0\1\0\0\0t\306\1\0004\0\0\0"..., 512) = 512 pread64(3, "\4\0\0\0\20\0\0\0\1\0\0\0GNU\0\0\0\0\0\3\0\0\0\2\0\0\0\0\0\0\0", 32, 1290836) = 32 mmap2(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = -1 EPERM (Operation not permitted) close(3) = 0 mmap2(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = -1 EPERM (Operation not permitted) writev(2, [{iov_base="./test", iov_len=6}, {iov_base=": ", iov_len=2}, {iov_base="error while loading shared libra"..., iov_len6}, {iov_base=": ", iov_len=2}, {iov_base="", iov_len=0}, {iov_base="", iov_len=0}, {iov_base="out of memory", iov_len\x13}, {iov_base=": ", iov_len=2}, {iov_base="Operation not permitted", iov_len#}, {iov_base="\n", iov_len=1}], 10./test: error while loading shared libraries: out of memory: Operation not permitted ) = 85 exit_group(127) = ? +++ exited with 127 +++ root@tirpitz:~> 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] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-08-28 15:50 ` Rich Felker 2020-08-28 16:21 ` John Paul Adrian Glaubitz @ 2020-08-28 16:30 ` Rich Felker 2020-08-28 16:38 ` John Paul Adrian Glaubitz 1 sibling, 1 reply; 20+ messages in thread From: Rich Felker @ 2020-08-28 16:30 UTC (permalink / raw) To: Michael Karcher; +Cc: linux-sh, linux-kernel, Yoshinori Sato, Adrian Glaubitz On Fri, Aug 28, 2020 at 11:50:25AM -0400, Rich Felker wrote: > On Thu, Jul 23, 2020 at 01:13:21AM +0200, Michael Karcher wrote: > > Port sh to use the new SECCOMP_FILTER code. > > > > Signed-off-by: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de> > > --- > > arch/sh/Kconfig | 1 + > > arch/sh/kernel/entry-common.S | 2 ++ > > arch/sh/kernel/ptrace_32.c | 5 +++-- > > tools/testing/selftests/seccomp/seccomp_bpf.c | 8 +++++++- > > 4 files changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig > > index 32d959849df9..10b510c16841 100644 > > --- a/arch/sh/Kconfig > > +++ b/arch/sh/Kconfig > > @@ -27,6 +27,7 @@ config SUPERH > > select GENERIC_SMP_IDLE_THREAD > > select GUP_GET_PTE_LOW_HIGH if X2TLB > > select HAVE_ARCH_AUDITSYSCALL > > + select HAVE_ARCH_SECCOMP_FILTER > > select HAVE_ARCH_KGDB > > select HAVE_ARCH_TRACEHOOK > > select HAVE_DEBUG_BUGVERBOSE > > diff --git a/arch/sh/kernel/entry-common.S b/arch/sh/kernel/entry-common.S > > index c4d88d61890d..ad963104d22d 100644 > > --- a/arch/sh/kernel/entry-common.S > > +++ b/arch/sh/kernel/entry-common.S > > @@ -368,6 +368,8 @@ syscall_trace_entry: > > mov.l 7f, r11 ! Call do_syscall_trace_enter which notifies > > jsr @r11 ! superior (will chomp R[0-7]) > > nop > > + cmp/eq #-1, r0 > > + bt syscall_exit > > mov.l r0, @(OFF_R0,r15) ! Save return value > > ! Reload R0-R4 from kernel stack, where the > > ! parent may have modified them using > > diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c > > index 64bfb714943e..25ccfbd02bfa 100644 > > --- a/arch/sh/kernel/ptrace_32.c > > +++ b/arch/sh/kernel/ptrace_32.c > > @@ -485,8 +485,6 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > > { > > long ret = 0; > > > > - secure_computing_strict(regs->regs[0]); > > - > > if (test_thread_flag(TIF_SYSCALL_TRACE) && > > tracehook_report_syscall_entry(regs)) > > /* > > @@ -496,6 +494,9 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > > */ > > ret = -1L; > > > > + if (secure_computing() = -1) > > + return -1; > > + > > if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) > > trace_sys_enter(regs, regs->regs[0]); > > > > This patch broke strace - it spews out bogus syscalls and gets the > tracee hung. I suspect the last hunk is wrong and breaks all > non-seccomp tracing. I'll follow up with further analysis and possibly > a fix if you don't find one sooner. It looks like the problem is actually the hunk in entry-common.S, but this code has been wrong since ab99c733ae in 2008: it was storing the return value of do_syscall_trace_enter, which is supposed to replace the syscall number and make it fail, in r0 (the 5th argument) rather than r3 (the syscall number). This looks like the reason you put the (apparently wrong) branch to syscall_exit in there -- the existing code was not actually causing ENOSYS when do_syscall_trace_enter tried to replace nr with -1, because the -1 was put in the wrong place. I'm guessing something in syscall_exit assumes the registers have been reloaded (the code skipped by your branch) and blows up when they haven't. I think the right change is going to be something like replacing mov.l r0, @(OFF_R0,r15) with mov r0, r3 and getting rid of the r3 reload below. do_syscall_trace_enter should also be returning regs->regs[3] in the success case, not regs->regs[0] as it's doing, at least if it's to match other archs (that return the original syscall number on success). In any case, returning the 5th argument register is nonsense. I'm about to test a patch along these lines and will report what I find. Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-08-28 16:30 ` Rich Felker @ 2020-08-28 16:38 ` John Paul Adrian Glaubitz 2020-08-28 17:03 ` Rich Felker 0 siblings, 1 reply; 20+ messages in thread From: John Paul Adrian Glaubitz @ 2020-08-28 16:38 UTC (permalink / raw) To: Rich Felker, Michael Karcher; +Cc: linux-sh, linux-kernel, Yoshinori Sato Hi! On 8/28/20 6:30 PM, Rich Felker wrote: > I'm about to test a patch along these lines and will report what I > find. Let me know when you have something to test and I will test the patch as well, making sure we're not breaking seccomp again. 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] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-08-28 16:38 ` John Paul Adrian Glaubitz @ 2020-08-28 17:03 ` Rich Felker 2020-08-29 0:49 ` Rich Felker 0 siblings, 1 reply; 20+ messages in thread From: Rich Felker @ 2020-08-28 17:03 UTC (permalink / raw) To: John Paul Adrian Glaubitz Cc: Michael Karcher, linux-sh, linux-kernel, Yoshinori Sato On Fri, Aug 28, 2020 at 06:38:09PM +0200, John Paul Adrian Glaubitz wrote: > Hi! > > On 8/28/20 6:30 PM, Rich Felker wrote: > > I'm about to test a patch along these lines and will report what I > > find. > > Let me know when you have something to test and I will test the patch as > well, making sure we're not breaking seccomp again. If you have a seccomp test setup, please try the following patch. I'm not sure if the end result is entirely correct, but I believe it's at least much closer to correct than the code was before or after adding SECCOMP_FILTER. diff --git a/arch/sh/kernel/entry-common.S b/arch/sh/kernel/entry-common.S index ad963104d22d..0560a8054215 100644 --- a/arch/sh/kernel/entry-common.S +++ b/arch/sh/kernel/entry-common.S @@ -368,9 +368,6 @@ syscall_trace_entry: mov.l 7f, r11 ! Call do_syscall_trace_enter which notifies jsr @r11 ! superior (will chomp R[0-7]) nop - cmp/eq #-1, r0 - bt syscall_exit - mov.l r0, @(OFF_R0,r15) ! Save return value ! Reload R0-R4 from kernel stack, where the ! parent may have modified them using ! ptrace(POKEUSR). (Note that R0-R2 are @@ -382,7 +379,7 @@ syscall_trace_entry: mov.l @(OFF_R5,r15), r5 mov.l @(OFF_R6,r15), r6 mov.l @(OFF_R7,r15), r7 ! arg3 - mov.l @(OFF_R3,r15), r3 ! syscall_nr + mov r0, r3 ! syscall_nr, possibly changed to -1 ! mov.l 6f, r10 ! Number of syscalls cmp/hs r10, r3 diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c index 25ccfbd02bfa..9e86cff041c7 100644 --- a/arch/sh/kernel/ptrace_32.c +++ b/arch/sh/kernel/ptrace_32.c @@ -503,7 +503,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) audit_syscall_entry(regs->regs[3], regs->regs[4], regs->regs[5], regs->regs[6], regs->regs[7]); - return ret ?: regs->regs[0]; + return ret ?: regs->regs[3]; } asmlinkage void do_syscall_trace_leave(struct pt_regs *regs) ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-08-28 17:03 ` Rich Felker @ 2020-08-29 0:49 ` Rich Felker 2020-08-29 11:09 ` John Paul Adrian Glaubitz 0 siblings, 1 reply; 20+ messages in thread From: Rich Felker @ 2020-08-29 0:49 UTC (permalink / raw) To: John Paul Adrian Glaubitz Cc: Michael Karcher, linux-sh, linux-kernel, Yoshinori Sato On Fri, Aug 28, 2020 at 01:03:00PM -0400, Rich Felker wrote: > On Fri, Aug 28, 2020 at 06:38:09PM +0200, John Paul Adrian Glaubitz wrote: > > Hi! > > > > On 8/28/20 6:30 PM, Rich Felker wrote: > > > I'm about to test a patch along these lines and will report what I > > > find. > > > > Let me know when you have something to test and I will test the patch as > > well, making sure we're not breaking seccomp again. > > If you have a seccomp test setup, please try the following patch. I'm > not sure if the end result is entirely correct, but I believe it's > at least much closer to correct than the code was before or after > adding SECCOMP_FILTER. > > > diff --git a/arch/sh/kernel/entry-common.S b/arch/sh/kernel/entry-common.S > index ad963104d22d..0560a8054215 100644 > --- a/arch/sh/kernel/entry-common.S > +++ b/arch/sh/kernel/entry-common.S > @@ -368,9 +368,6 @@ syscall_trace_entry: > mov.l 7f, r11 ! Call do_syscall_trace_enter which notifies > jsr @r11 ! superior (will chomp R[0-7]) > nop > - cmp/eq #-1, r0 > - bt syscall_exit > - mov.l r0, @(OFF_R0,r15) ! Save return value > ! Reload R0-R4 from kernel stack, where the > ! parent may have modified them using > ! ptrace(POKEUSR). (Note that R0-R2 are > @@ -382,7 +379,7 @@ syscall_trace_entry: > mov.l @(OFF_R5,r15), r5 > mov.l @(OFF_R6,r15), r6 > mov.l @(OFF_R7,r15), r7 ! arg3 > - mov.l @(OFF_R3,r15), r3 ! syscall_nr > + mov r0, r3 ! syscall_nr, possibly changed to -1 > ! > mov.l 6f, r10 ! Number of syscalls > cmp/hs r10, r3 > diff --git a/arch/sh/kernel/ptrace_32.c b/arch/sh/kernel/ptrace_32.c > index 25ccfbd02bfa..9e86cff041c7 100644 > --- a/arch/sh/kernel/ptrace_32.c > +++ b/arch/sh/kernel/ptrace_32.c > @@ -503,7 +503,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs) > audit_syscall_entry(regs->regs[3], regs->regs[4], regs->regs[5], > regs->regs[6], regs->regs[7]); > > - return ret ?: regs->regs[0]; > + return ret ?: regs->regs[3]; > } > > asmlinkage void do_syscall_trace_leave(struct pt_regs *regs) This restored my ability to use strace, and I've written and tested a minimal strace-like hack using SECCOMP_RET_USER_NOTIF that works as expected on both j2 and qemu-system-sh4, so I think the above is correct. Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-08-29 0:49 ` Rich Felker @ 2020-08-29 11:09 ` John Paul Adrian Glaubitz 2020-09-03 3:56 ` Rich Felker 0 siblings, 1 reply; 20+ messages in thread From: John Paul Adrian Glaubitz @ 2020-08-29 11:09 UTC (permalink / raw) To: Rich Felker; +Cc: Michael Karcher, linux-sh, linux-kernel, Yoshinori Sato Hi! On 8/29/20 2:49 AM, Rich Felker wrote: > This restored my ability to use strace I can confirm that. However ... > and I've written and tested a minimal strace-like hack using > SECCOMP_RET_USER_NOTIF that works as > expected on both j2 and qemu-system-sh4, so I think the above is > correct. The seccomp live testsuite has regressed. With your patch: ======== Sat 29 Aug 2020 12:35:52 PM CEST =======Regression Test Report ("regression -T live") batch name: 01-sim-allow test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 02-sim-basic test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 03-sim-basic_chains test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 04-sim-multilevel_chains test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 05-sim-long_jumps test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 06-sim-actions test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 07-sim-db_bug_looping test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 08-sim-subtree_checks test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 09-sim-syscall_priority_pre test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 10-sim-syscall_priority_post test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 11-basic-basic_errors test mode: c test type: basic batch name: 12-sim-basic_masked_ops test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 13-basic-attrs test mode: c test type: basic batch name: 14-sim-reset test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 15-basic-resolver test mode: c test type: basic batch name: 16-sim-arch_basic test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 17-sim-arch_merge test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 18-sim-basic_allowlist test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 19-sim-missing_syscalls test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 20-live-basic_die test mode: c test type: live Test 20-live-basic_die%%001-00001 result: SUCCESS Test 20-live-basic_die%%002-00001 result: SUCCESS Test 20-live-basic_die%%003-00001 result: FAILURE 20-live-basic_die 1 ERRNO rc8 batch name: 21-live-basic_allow test mode: c test type: live Test 21-live-basic_allow%%001-00001 result: SUCCESS batch name: 22-sim-basic_chains_array test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 23-sim-arch_all_le_basic test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 24-live-arg_allow test mode: c test type: live Test 24-live-arg_allow%%001-00001 result: SUCCESS batch name: 25-sim-multilevel_chains_adv test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 26-sim-arch_all_be_basic test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 27-sim-bpf_blk_state test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 28-sim-arch_x86 test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 29-sim-pseudo_syscall test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 30-sim-socket_syscalls test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 31-basic-version_check test mode: c test type: basic batch name: 32-live-tsync_allow test mode: c test type: live Test 32-live-tsync_allow%%001-00001 result: SUCCESS batch name: 33-sim-socket_syscalls_be test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 34-sim-basic_denylist test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 35-sim-negative_one test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 36-sim-ipc_syscalls test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 37-sim-ipc_syscalls_be test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 38-basic-pfc_coverage test mode: c test type: basic batch name: 39-basic-api_level test mode: c test type: basic batch name: 40-sim-log test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 41-sim-syscall_priority_arch test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 42-sim-adv_chains test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 43-sim-a2_order test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 44-live-a2_order test mode: c test type: live Test 44-live-a2_order%%001-00001 result: FAILURE 44-live-a2_order 1 ALLOW rc=1 batch name: 45-sim-chain_code_coverage test mode: c test type: bpf-sim batch name: 46-sim-kill_process test mode: c test type: bpf-sim batch name: 47-live-kill_process test mode: c test type: live Test 47-live-kill_process%%001-00001 result: SUCCESS batch name: 48-sim-32b_args test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 49-sim-64b_comparisons test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 50-sim-hash_collision test mode: c test type: bpf-sim batch name: 51-live-user_notification test mode: c test type: live Test 51-live-user_notification%%001-00001 result: FAILURE 51-live-user_notification 5 ALLOW rc\x14 batch name: 52-basic-load test mode: c test type: basic batch name: 53-sim-binary_tree test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 54-live-binary_tree test mode: c test type: live Test 54-live-binary_tree%%001-00001 result: SUCCESS batch name: 55-basic-pfc_binary_tree test mode: c test type: basic batch name: 56-basic-iterate_syscalls test mode: c test type: basic batch name: 57-basic-rawsysrc test mode: c test type: basic batch name: 58-live-tsync_notify test mode: c test type: live Test 58-live-tsync_notify%%001-00001 result: FAILURE 58-live-tsync_notify 6 ALLOW rc\x14 Regression Test Summary tests run: 11 tests skipped: 0 tests passed: 7 tests failed: 4 tests errored: 0 ============================== And without: ======== Sat 29 Aug 2020 01:03:07 PM CEST =======Regression Test Report ("regression -T live") batch name: 01-sim-allow test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 02-sim-basic test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 03-sim-basic_chains test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 04-sim-multilevel_chains test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 05-sim-long_jumps test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 06-sim-actions test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 07-sim-db_bug_looping test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 08-sim-subtree_checks test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 09-sim-syscall_priority_pre test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 10-sim-syscall_priority_post test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 11-basic-basic_errors test mode: c test type: basic batch name: 12-sim-basic_masked_ops test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 13-basic-attrs test mode: c test type: basic batch name: 14-sim-reset test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 15-basic-resolver test mode: c test type: basic batch name: 16-sim-arch_basic test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 17-sim-arch_merge test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 18-sim-basic_allowlist test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 19-sim-missing_syscalls test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 20-live-basic_die test mode: c test type: live Test 20-live-basic_die%%001-00001 result: SUCCESS Test 20-live-basic_die%%002-00001 result: SUCCESS Test 20-live-basic_die%%003-00001 result: SUCCESS batch name: 21-live-basic_allow test mode: c test type: live Test 21-live-basic_allow%%001-00001 result: SUCCESS batch name: 22-sim-basic_chains_array test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 23-sim-arch_all_le_basic test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 24-live-arg_allow test mode: c test type: live Test 24-live-arg_allow%%001-00001 result: SUCCESS batch name: 25-sim-multilevel_chains_adv test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 26-sim-arch_all_be_basic test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 27-sim-bpf_blk_state test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 28-sim-arch_x86 test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 29-sim-pseudo_syscall test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 30-sim-socket_syscalls test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 31-basic-version_check test mode: c test type: basic batch name: 32-live-tsync_allow test mode: c test type: live Test 32-live-tsync_allow%%001-00001 result: SUCCESS batch name: 33-sim-socket_syscalls_be test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 34-sim-basic_denylist test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 35-sim-negative_one test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 36-sim-ipc_syscalls test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 37-sim-ipc_syscalls_be test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 38-basic-pfc_coverage test mode: c test type: basic batch name: 39-basic-api_level test mode: c test type: basic batch name: 40-sim-log test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 41-sim-syscall_priority_arch test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 42-sim-adv_chains test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 43-sim-a2_order test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 44-live-a2_order test mode: c test type: live Test 44-live-a2_order%%001-00001 result: SUCCESS batch name: 45-sim-chain_code_coverage test mode: c test type: bpf-sim batch name: 46-sim-kill_process test mode: c test type: bpf-sim batch name: 47-live-kill_process test mode: c test type: live Test 47-live-kill_process%%001-00001 result: SUCCESS batch name: 48-sim-32b_args test mode: c test type: bpf-sim test mode: c test type: bpf-sim-fuzz test mode: c test type: bpf-valgrind batch name: 49-sim-64b_comparisons test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 50-sim-hash_collision test mode: c test type: bpf-sim batch name: 51-live-user_notification test mode: c test type: live Test 51-live-user_notification%%001-00001 result: SUCCESS batch name: 52-basic-load test mode: c test type: basic batch name: 53-sim-binary_tree test mode: c test type: bpf-sim test mode: c test type: bpf-valgrind batch name: 54-live-binary_tree test mode: c test type: live Test 54-live-binary_tree%%001-00001 result: SUCCESS batch name: 55-basic-pfc_binary_tree test mode: c test type: basic batch name: 56-basic-iterate_syscalls test mode: c test type: basic batch name: 57-basic-rawsysrc test mode: c test type: basic batch name: 58-live-tsync_notify test mode: c test type: live Test 58-live-tsync_notify%%001-00001 result: SUCCESS Regression Test Summary tests run: 11 tests skipped: 0 tests passed: 11 tests failed: 0 tests errored: 0 ============================== To test libseccomp, check out my superh branch from here: > https://github.com/glaubitz/libseccomp/tree/superh then build and test with: # ./autogen.sh && ./configure && make && make check && make check-build && cd tests && ./regression -T live Maybe Michael Karcher has any idea what's wrong with the strace stuff? 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] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-08-29 11:09 ` John Paul Adrian Glaubitz @ 2020-09-03 3:56 ` Rich Felker 2020-09-03 5:46 ` Rich Felker 2020-09-03 6:03 ` John Paul Adrian Glaubitz 0 siblings, 2 replies; 20+ messages in thread From: Rich Felker @ 2020-09-03 3:56 UTC (permalink / raw) To: John Paul Adrian Glaubitz Cc: Michael Karcher, linux-sh, linux-kernel, Yoshinori Sato On Sat, Aug 29, 2020 at 01:09:43PM +0200, John Paul Adrian Glaubitz wrote: > Hi! > > On 8/29/20 2:49 AM, Rich Felker wrote: > > This restored my ability to use strace > > I can confirm that. However ... > > > and I've written and tested a minimal strace-like hack using > > SECCOMP_RET_USER_NOTIF that works as > > expected on both j2 and qemu-system-sh4, so I think the above is > > correct. > > The seccomp live testsuite has regressed. > > With your patch: > > ======== Sat 29 Aug 2020 12:35:52 PM CEST =======> Regression Test Report ("regression -T live") > batch name: 01-sim-allow > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 02-sim-basic > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 03-sim-basic_chains > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 04-sim-multilevel_chains > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 05-sim-long_jumps > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 06-sim-actions > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 07-sim-db_bug_looping > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 08-sim-subtree_checks > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 09-sim-syscall_priority_pre > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 10-sim-syscall_priority_post > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 11-basic-basic_errors > test mode: c > test type: basic > batch name: 12-sim-basic_masked_ops > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 13-basic-attrs > test mode: c > test type: basic > batch name: 14-sim-reset > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 15-basic-resolver > test mode: c > test type: basic > batch name: 16-sim-arch_basic > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 17-sim-arch_merge > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 18-sim-basic_allowlist > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 19-sim-missing_syscalls > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 20-live-basic_die > test mode: c > test type: live > Test 20-live-basic_die%%001-00001 result: SUCCESS > Test 20-live-basic_die%%002-00001 result: SUCCESS > Test 20-live-basic_die%%003-00001 result: FAILURE 20-live-basic_die 1 ERRNO rc8 > batch name: 21-live-basic_allow > test mode: c > test type: live > Test 21-live-basic_allow%%001-00001 result: SUCCESS > batch name: 22-sim-basic_chains_array > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 23-sim-arch_all_le_basic > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 24-live-arg_allow > test mode: c > test type: live > Test 24-live-arg_allow%%001-00001 result: SUCCESS > batch name: 25-sim-multilevel_chains_adv > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 26-sim-arch_all_be_basic > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 27-sim-bpf_blk_state > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 28-sim-arch_x86 > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 29-sim-pseudo_syscall > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 30-sim-socket_syscalls > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 31-basic-version_check > test mode: c > test type: basic > batch name: 32-live-tsync_allow > test mode: c > test type: live > Test 32-live-tsync_allow%%001-00001 result: SUCCESS > batch name: 33-sim-socket_syscalls_be > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 34-sim-basic_denylist > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 35-sim-negative_one > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 36-sim-ipc_syscalls > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 37-sim-ipc_syscalls_be > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 38-basic-pfc_coverage > test mode: c > test type: basic > batch name: 39-basic-api_level > test mode: c > test type: basic > batch name: 40-sim-log > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 41-sim-syscall_priority_arch > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 42-sim-adv_chains > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 43-sim-a2_order > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 44-live-a2_order > test mode: c > test type: live > Test 44-live-a2_order%%001-00001 result: FAILURE 44-live-a2_order 1 ALLOW rc=1 > batch name: 45-sim-chain_code_coverage > test mode: c > test type: bpf-sim > batch name: 46-sim-kill_process > test mode: c > test type: bpf-sim > batch name: 47-live-kill_process > test mode: c > test type: live > Test 47-live-kill_process%%001-00001 result: SUCCESS > batch name: 48-sim-32b_args > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-sim-fuzz > test mode: c > test type: bpf-valgrind > batch name: 49-sim-64b_comparisons > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 50-sim-hash_collision > test mode: c > test type: bpf-sim > batch name: 51-live-user_notification > test mode: c > test type: live > Test 51-live-user_notification%%001-00001 result: FAILURE 51-live-user_notification 5 ALLOW rc\x14 AFAICT, this test is buggy and cannot possibly work. It attempts to have SYS_getpid return a 64-bit value and check that the returned value matches. On 32-bit archs this will be truncated to 32 bits, but the comparison in the caller still compares against the full 64-bit value. I have no idea how this seemed to work before. > batch name: 52-basic-load > test mode: c > test type: basic > batch name: 53-sim-binary_tree > test mode: c > test type: bpf-sim > test mode: c > test type: bpf-valgrind > batch name: 54-live-binary_tree > test mode: c > test type: live > Test 54-live-binary_tree%%001-00001 result: SUCCESS > batch name: 55-basic-pfc_binary_tree > test mode: c > test type: basic > batch name: 56-basic-iterate_syscalls > test mode: c > test type: basic > batch name: 57-basic-rawsysrc > test mode: c > test type: basic > batch name: 58-live-tsync_notify > test mode: c > test type: live > Test 58-live-tsync_notify%%001-00001 result: FAILURE 58-live-tsync_notify 6 ALLOW rc\x14 This is similar to 51. I think the commonality of all the failures is that they deal with return values set by seccomp filters for blocked syscalls, which are getting clobbered by ENOSYS from the failed syscall here. So I do need to keep the code path that jumps over the actual syscall if do_syscall_trace_enter returns -1, but that means do_syscall_trace_enter must now be responsible for setting the return value in non-seccomp failure paths. I'll experiment to see what's still needed if that change is made. > [...] > ============================== > > To test libseccomp, check out my superh branch from here: > > > https://github.com/glaubitz/libseccomp/tree/superh > > then build and test with: > > # ./autogen.sh && ./configure && make && make check && make check-build && cd tests && ./regression -T live > > Maybe Michael Karcher has any idea what's wrong with the strace stuff? I'd welcome any input, but I think I'm on track to solving this either way. Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-09-03 3:56 ` Rich Felker @ 2020-09-03 5:46 ` Rich Felker 2020-09-03 6:04 ` John Paul Adrian Glaubitz 2020-09-03 6:03 ` John Paul Adrian Glaubitz 1 sibling, 1 reply; 20+ messages in thread From: Rich Felker @ 2020-09-03 5:46 UTC (permalink / raw) To: John Paul Adrian Glaubitz Cc: Michael Karcher, linux-sh, linux-kernel, Yoshinori Sato On Wed, Sep 02, 2020 at 11:56:04PM -0400, Rich Felker wrote: > On Sat, Aug 29, 2020 at 01:09:43PM +0200, John Paul Adrian Glaubitz wrote: > > Hi! > > > > On 8/29/20 2:49 AM, Rich Felker wrote: > > > This restored my ability to use strace > > > > I can confirm that. However ... > > > > > and I've written and tested a minimal strace-like hack using > > > SECCOMP_RET_USER_NOTIF that works as > > > expected on both j2 and qemu-system-sh4, so I think the above is > > > correct. > > > > The seccomp live testsuite has regressed. > > > > [...] > > Test 58-live-tsync_notify%%001-00001 result: FAILURE 58-live-tsync_notify 6 ALLOW rc\x14 > > This is similar to 51. > > I think the commonality of all the failures is that they deal with > return values set by seccomp filters for blocked syscalls, which are > getting clobbered by ENOSYS from the failed syscall here. So I do need > to keep the code path that jumps over the actual syscall if > do_syscall_trace_enter returns -1, but that means > do_syscall_trace_enter must now be responsible for setting the return > value in non-seccomp failure paths. > > I'll experiment to see what's still needed if that change is made. OK, I think I have an explanation for the mechanism of the bug, and it really is a combination of the 2008 bug (confusion of r0 vs r3) and the SECCOMP_FILTER commit. When the syscall_trace_entry code path is in use, a syscall with argument 5 having value -1 causes do_syscall_trace_enter to return -1 (because it returns regs[0], which contains argument 5), which the change in entry-common.S interprets as a sign to skip the syscall and jump to syscall_exit, and things blow up from there. In particular, SYS_mmap2 is almost always called with -1 as the 5th argument (fd), and this is even more common on nommu where SYS_brk does not work. I'll follow up with a new proposed patch. Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-09-03 5:46 ` Rich Felker @ 2020-09-03 6:04 ` John Paul Adrian Glaubitz 2020-09-03 6:17 ` Rich Felker 0 siblings, 1 reply; 20+ messages in thread From: John Paul Adrian Glaubitz @ 2020-09-03 6:04 UTC (permalink / raw) To: Rich Felker; +Cc: Michael Karcher, linux-sh, linux-kernel, Yoshinori Sato On 9/3/20 7:46 AM, Rich Felker wrote: > > OK, I think I have an explanation for the mechanism of the bug, and it > really is a combination of the 2008 bug (confusion of r0 vs r3) and > the SECCOMP_FILTER commit. When the syscall_trace_entry code path is > in use, a syscall with argument 5 having value -1 causes > do_syscall_trace_enter to return -1 (because it returns regs[0], which > contains argument 5), which the change in entry-common.S interprets as > a sign to skip the syscall and jump to syscall_exit, and things blow > up from there. In particular, SYS_mmap2 is almost always called with > -1 as the 5th argument (fd), and this is even more common on nommu > where SYS_brk does not work. > > I'll follow up with a new proposed patch. I'm not sure whether we need another revision of your first patch. Your previous analysis was at least right regarding the tests 51 and 58 but those have been fixed now. But there were two other tests failing, weren't there? I have to recheck later, I just got up (it's 8 AM CEST). 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] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-09-03 6:04 ` John Paul Adrian Glaubitz @ 2020-09-03 6:17 ` Rich Felker 0 siblings, 0 replies; 20+ messages in thread From: Rich Felker @ 2020-09-03 6:17 UTC (permalink / raw) To: John Paul Adrian Glaubitz Cc: Michael Karcher, linux-sh, linux-kernel, Yoshinori Sato On Thu, Sep 03, 2020 at 08:04:44AM +0200, John Paul Adrian Glaubitz wrote: > On 9/3/20 7:46 AM, Rich Felker wrote: > > > > OK, I think I have an explanation for the mechanism of the bug, and it > > really is a combination of the 2008 bug (confusion of r0 vs r3) and > > the SECCOMP_FILTER commit. When the syscall_trace_entry code path is > > in use, a syscall with argument 5 having value -1 causes > > do_syscall_trace_enter to return -1 (because it returns regs[0], which > > contains argument 5), which the change in entry-common.S interprets as > > a sign to skip the syscall and jump to syscall_exit, and things blow > > up from there. In particular, SYS_mmap2 is almost always called with > > -1 as the 5th argument (fd), and this is even more common on nommu > > where SYS_brk does not work. > > > > I'll follow up with a new proposed patch. > > I'm not sure whether we need another revision of your first patch. Your > previous analysis was at least right regarding the tests 51 and 58 > but those have been fixed now. > > But there were two other tests failing, weren't there? > > I have to recheck later, I just got up (it's 8 AM CEST). The first patch was surely not right; setting syscall_nr to -1 and letting it -ENOSYS clobbered any return value set by the seccomp filters. The one I've sent now should be right. I'll follow up after testing with libseccomp test cases. Rich ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] sh: Add SECCOMP_FILTER 2020-09-03 3:56 ` Rich Felker 2020-09-03 5:46 ` Rich Felker @ 2020-09-03 6:03 ` John Paul Adrian Glaubitz 1 sibling, 0 replies; 20+ messages in thread From: John Paul Adrian Glaubitz @ 2020-09-03 6:03 UTC (permalink / raw) To: Rich Felker; +Cc: Michael Karcher, linux-sh, linux-kernel, Yoshinori Sato Hi Richi! On 9/3/20 5:56 AM, Rich Felker wrote: >> Test 51-live-user_notification%%001-00001 result: FAILURE 51-live-user_notification 5 ALLOW rc\x14 > > AFAICT, this test is buggy and cannot possibly work. It attempts to > have SYS_getpid return a 64-bit value and check that the returned > value matches. On 32-bit archs this will be truncated to 32 bits, but > the comparison in the caller still compares against the full 64-bit > value. I have no idea how this seemed to work before. You're actually right, I forgot about that. Michael discovered this bug as well and it was consequently fixed: > https://github.com/seccomp/libseccomp/commit/bee43d3e884788569860a384e6a38357785a3995 >> Test 58-live-tsync_notify%%001-00001 result: FAILURE 58-live-tsync_notify 6 ALLOW rc\x14 > > This is similar to 51. > > I think the commonality of all the failures is that they deal with > return values set by seccomp filters for blocked syscalls, which are > getting clobbered by ENOSYS from the failed syscall here. So I do need > to keep the code path that jumps over the actual syscall if > do_syscall_trace_enter returns -1, but that means > do_syscall_trace_enter must now be responsible for setting the return > value in non-seccomp failure paths. Same here: > https://github.com/seccomp/libseccomp/commit/f0686d9de911e7ffcdc7364566c1d146e44657c2 Not sure about the other two tests. I can re-base and re-test. 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] 20+ messages in thread
* [PATCH 4/4] sh: bring syscall_set_return_value in line with other architectures 2020-07-22 23:13 [PATCH 1/4] sh: Fix validation of system call number Michael Karcher 2020-07-22 23:13 ` [PATCH 2/4] sh: Rearrange blocks in entry-common.S Michael Karcher 2020-07-22 23:13 ` [PATCH 3/4] sh: Add SECCOMP_FILTER Michael Karcher @ 2020-07-22 23:13 ` Michael Karcher 2020-07-22 23:20 ` John Paul Adrian Glaubitz 2020-07-22 23:19 ` [PATCH 1/4] sh: Fix validation of system call number John Paul Adrian Glaubitz 3 siblings, 1 reply; 20+ messages in thread From: Michael Karcher @ 2020-07-22 23:13 UTC (permalink / raw) To: linux-sh, linux-kernel Cc: Yoshinori Sato, Rich Felker, Adrian Glaubitz, Michael Karcher Other architectures expect that syscall_set_return_value gets an already negative value as error. That's also what kernel/seccomp.c provides. Signed-off-by: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de> --- arch/sh/include/asm/syscall_32.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/arch/sh/include/asm/syscall_32.h b/arch/sh/include/asm/syscall_32.h index 0b5b8e75edac..cb51a7528384 100644 --- a/arch/sh/include/asm/syscall_32.h +++ b/arch/sh/include/asm/syscall_32.h @@ -40,10 +40,7 @@ static inline void syscall_set_return_value(struct task_struct *task, struct pt_regs *regs, int error, long val) { - if (error) - regs->regs[0] = -error; - else - regs->regs[0] = val; + regs->regs[0] = (long) error ?: val; } static inline void syscall_get_arguments(struct task_struct *task, -- 2.28.0.rc1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] sh: bring syscall_set_return_value in line with other architectures 2020-07-22 23:13 ` [PATCH 4/4] sh: bring syscall_set_return_value in line with other architectures Michael Karcher @ 2020-07-22 23:20 ` John Paul Adrian Glaubitz 0 siblings, 0 replies; 20+ messages in thread From: John Paul Adrian Glaubitz @ 2020-07-22 23:20 UTC (permalink / raw) To: Michael Karcher, linux-sh, linux-kernel; +Cc: Yoshinori Sato, Rich Felker On 7/23/20 1:13 AM, Michael Karcher wrote: > Other architectures expect that syscall_set_return_value gets an already > negative value as error. That's also what kernel/seccomp.c provides. > > Signed-off-by: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de> > --- > arch/sh/include/asm/syscall_32.h | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/arch/sh/include/asm/syscall_32.h b/arch/sh/include/asm/syscall_32.h > index 0b5b8e75edac..cb51a7528384 100644 > --- a/arch/sh/include/asm/syscall_32.h > +++ b/arch/sh/include/asm/syscall_32.h > @@ -40,10 +40,7 @@ static inline void syscall_set_return_value(struct task_struct *task, > struct pt_regs *regs, > int error, long val) > { > - if (error) > - regs->regs[0] = -error; > - else > - regs->regs[0] = val; > + regs->regs[0] = (long) error ?: val; > } > > static inline void syscall_get_arguments(struct task_struct *task, > Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> -- .''`. 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] 20+ messages in thread
* Re: [PATCH 1/4] sh: Fix validation of system call number 2020-07-22 23:13 [PATCH 1/4] sh: Fix validation of system call number Michael Karcher ` (2 preceding siblings ...) 2020-07-22 23:13 ` [PATCH 4/4] sh: bring syscall_set_return_value in line with other architectures Michael Karcher @ 2020-07-22 23:19 ` John Paul Adrian Glaubitz 3 siblings, 0 replies; 20+ messages in thread From: John Paul Adrian Glaubitz @ 2020-07-22 23:19 UTC (permalink / raw) To: Michael Karcher, linux-sh, linux-kernel; +Cc: Yoshinori Sato, Rich Felker On 7/23/20 1:13 AM, Michael Karcher wrote: > The slow path for traced system call entries accessed a wrong memory > location to get the number of the maximum allowed system call number. > Renumber the numbered "local" label for the correct location to avoid > collisions with actual local labels. > > Signed-off-by: Michael Karcher <kernel@mkarcher.dialup.fu-berlin.de> > --- > arch/sh/kernel/entry-common.S | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/sh/kernel/entry-common.S b/arch/sh/kernel/entry-common.S > index 956a7a03b0c8..9bac5bbb67f3 100644 > --- a/arch/sh/kernel/entry-common.S > +++ b/arch/sh/kernel/entry-common.S > @@ -199,7 +199,7 @@ syscall_trace_entry: > mov.l @(OFF_R7,r15), r7 ! arg3 > mov.l @(OFF_R3,r15), r3 ! syscall_nr > ! > - mov.l 2f, r10 ! Number of syscalls > + mov.l 6f, r10 ! Number of syscalls > cmp/hs r10, r3 > bf syscall_call > mov #-ENOSYS, r0 > @@ -353,7 +353,7 @@ ENTRY(system_call) > tst r9, r8 > bf syscall_trace_entry > ! > - mov.l 2f, r8 ! Number of syscalls > + mov.l 6f, r8 ! Number of syscalls > cmp/hs r8, r3 > bt syscall_badsys > ! > @@ -392,7 +392,7 @@ syscall_exit: > #if !defined(CONFIG_CPU_SH2) > 1: .long TRA > #endif > -2: .long NR_syscalls > +6: .long NR_syscalls > 3: .long sys_call_table > 7: .long do_syscall_trace_enter > 8: .long do_syscall_trace_leave > Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> -- .''`. 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] 20+ messages in thread
end of thread, other threads:[~2020-09-03 6:17 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-22 23:13 [PATCH 1/4] sh: Fix validation of system call number Michael Karcher 2020-07-22 23:13 ` [PATCH 2/4] sh: Rearrange blocks in entry-common.S Michael Karcher 2020-07-22 23:20 ` John Paul Adrian Glaubitz 2020-07-22 23:13 ` [PATCH 3/4] sh: Add SECCOMP_FILTER Michael Karcher 2020-07-22 23:20 ` John Paul Adrian Glaubitz 2020-08-28 15:50 ` Rich Felker 2020-08-28 16:21 ` John Paul Adrian Glaubitz 2020-08-28 16:30 ` Rich Felker 2020-08-28 16:38 ` John Paul Adrian Glaubitz 2020-08-28 17:03 ` Rich Felker 2020-08-29 0:49 ` Rich Felker 2020-08-29 11:09 ` John Paul Adrian Glaubitz 2020-09-03 3:56 ` Rich Felker 2020-09-03 5:46 ` Rich Felker 2020-09-03 6:04 ` John Paul Adrian Glaubitz 2020-09-03 6:17 ` Rich Felker 2020-09-03 6:03 ` John Paul Adrian Glaubitz 2020-07-22 23:13 ` [PATCH 4/4] sh: bring syscall_set_return_value in line with other architectures Michael Karcher 2020-07-22 23:20 ` John Paul Adrian Glaubitz 2020-07-22 23:19 ` [PATCH 1/4] sh: Fix validation of system call number John Paul Adrian Glaubitz
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).