* [PATCH] ARM: Call syscall_trace_exit even when system call skipped @ 2018-02-03 15:21 Timothy E Baldwin 2018-03-13 23:58 ` Dmitry V. Levin ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Timothy E Baldwin @ 2018-02-03 15:21 UTC (permalink / raw) To: linux-arm-kernel On at least x86 and ARM64, and as documented in the ptrace man page a skipped system call will still cause a syscall exit ptrace stop. Previous to this commit 32-bit ARM did not, resulting in strace being confused when seccomp skips system calls. This change also impacts programs that use ptrace to skip system calls. Fixes: ad75b51459ae ("ARM: 7579/1: arch/allow a scno of -1 to not cause a SIGILL") Signed-off-by: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk> --- arch/arm/kernel/entry-common.S | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S index 99c908226065..88a65157307d 100644 --- a/arch/arm/kernel/entry-common.S +++ b/arch/arm/kernel/entry-common.S @@ -288,16 +288,15 @@ __sys_trace: cmp scno, #-1 @ skip the syscall? bne 2b add sp, sp, #S_OFF @ restore stack - b ret_slow_syscall -__sys_trace_return: - str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 +__sys_trace_return_nosave: + enable_irq_notrace mov r0, sp bl syscall_trace_exit b ret_slow_syscall -__sys_trace_return_nosave: - enable_irq_notrace +__sys_trace_return: + str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 mov r0, sp bl syscall_trace_exit b ret_slow_syscall -- 2.14.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* ARM: Call syscall_trace_exit even when system call skipped 2018-02-03 15:21 [PATCH] ARM: Call syscall_trace_exit even when system call skipped Timothy E Baldwin @ 2018-03-13 23:58 ` Dmitry V. Levin 2018-03-15 0:44 ` Kees Cook 2018-10-08 11:00 ` Eugene Syromyatnikov 2018-10-08 18:26 ` Eugene Syromyatnikov 2 siblings, 1 reply; 13+ messages in thread From: Dmitry V. Levin @ 2018-03-13 23:58 UTC (permalink / raw) To: linux-arm-kernel Hi Kees, As you probably know, ptracing of processes affected by SECCOMP_RET_TRAP is broken on ARM since your commit v3.7-rc1-11-gad75b51459ae. Could you review the proposed fix, please? P.S. There is a test for this kernel bug in strace test suite, you might find it useful: https://github.com/strace/strace/compare/ldv/SECCOMP_RET_TRAP On Sat, Feb 03, 2018 at 03:21:12PM +0000, Timothy E Baldwin wrote: > On at least x86 and ARM64, and as documented in the ptrace man page > a skipped system call will still cause a syscall exit ptrace stop. > > Previous to this commit 32-bit ARM did not, resulting in strace > being confused when seccomp skips system calls. > > This change also impacts programs that use ptrace to skip system calls. > > Fixes: ad75b51459ae ("ARM: 7579/1: arch/allow a scno of -1 to not cause a SIGILL") > Signed-off-by: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk> > --- > arch/arm/kernel/entry-common.S | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S > index 99c908226065..88a65157307d 100644 > --- a/arch/arm/kernel/entry-common.S > +++ b/arch/arm/kernel/entry-common.S > @@ -288,16 +288,15 @@ __sys_trace: > cmp scno, #-1 @ skip the syscall? > bne 2b > add sp, sp, #S_OFF @ restore stack > - b ret_slow_syscall > > -__sys_trace_return: > - str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 > +__sys_trace_return_nosave: > + enable_irq_notrace > mov r0, sp > bl syscall_trace_exit > b ret_slow_syscall > > -__sys_trace_return_nosave: > - enable_irq_notrace > +__sys_trace_return: > + str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 > mov r0, sp > bl syscall_trace_exit > b ret_slow_syscall -- ldv -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 801 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180314/f540b10f/attachment.sig> ^ permalink raw reply [flat|nested] 13+ messages in thread
* ARM: Call syscall_trace_exit even when system call skipped 2018-03-13 23:58 ` Dmitry V. Levin @ 2018-03-15 0:44 ` Kees Cook 2018-03-15 10:38 ` T.E.Baldwin99 at members.leeds.ac.uk 0 siblings, 1 reply; 13+ messages in thread From: Kees Cook @ 2018-03-15 0:44 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 13, 2018 at 4:58 PM, Dmitry V. Levin <ldv@altlinux.org> wrote: > Hi Kees, > > As you probably know, ptracing of processes affected by SECCOMP_RET_TRAP > is broken on ARM since your commit v3.7-rc1-11-gad75b51459ae. Hi! Well that's no good. I hadn't seen this problem; the seccomp selftests don't trip over this. > Could you review the proposed fix, please? The seccomp selftests still pass with the proposed fix, so that's all fine by me. :) > P.S. There is a test for this kernel bug in strace test suite, > you might find it useful: > https://github.com/strace/strace/compare/ldv/SECCOMP_RET_TRAP > > On Sat, Feb 03, 2018 at 03:21:12PM +0000, Timothy E Baldwin wrote: >> On at least x86 and ARM64, and as documented in the ptrace man page >> a skipped system call will still cause a syscall exit ptrace stop. >> >> Previous to this commit 32-bit ARM did not, resulting in strace >> being confused when seccomp skips system calls. >> >> This change also impacts programs that use ptrace to skip system calls. >> >> Fixes: ad75b51459ae ("ARM: 7579/1: arch/allow a scno of -1 to not cause a SIGILL") >> Signed-off-by: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk> >> --- >> arch/arm/kernel/entry-common.S | 9 ++++----- >> 1 file changed, 4 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S >> index 99c908226065..88a65157307d 100644 >> --- a/arch/arm/kernel/entry-common.S >> +++ b/arch/arm/kernel/entry-common.S >> @@ -288,16 +288,15 @@ __sys_trace: >> cmp scno, #-1 @ skip the syscall? >> bne 2b >> add sp, sp, #S_OFF @ restore stack >> - b ret_slow_syscall >> >> -__sys_trace_return: >> - str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 >> +__sys_trace_return_nosave: >> + enable_irq_notrace Why is __sys_trace_return_nosave the correct destination here? The original handle set up for lr a few lines above is for __sys_trace_return. It's not clear to me why this change is made? >> mov r0, sp >> bl syscall_trace_exit >> b ret_slow_syscall >> >> -__sys_trace_return_nosave: >> - enable_irq_notrace >> +__sys_trace_return: >> + str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 >> mov r0, sp >> bl syscall_trace_exit >> b ret_slow_syscall > > -- > ldv Otherwise, I think this looks good. -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 13+ messages in thread
* ARM: Call syscall_trace_exit even when system call skipped 2018-03-15 0:44 ` Kees Cook @ 2018-03-15 10:38 ` T.E.Baldwin99 at members.leeds.ac.uk 2018-03-15 20:14 ` Kees Cook 0 siblings, 1 reply; 13+ messages in thread From: T.E.Baldwin99 at members.leeds.ac.uk @ 2018-03-15 10:38 UTC (permalink / raw) To: linux-arm-kernel On 15 March 2018 00:45:01 Kees Cook <keescook@chromium.org> wrote: >>> --- a/arch/arm/kernel/entry-common.S >>> +++ b/arch/arm/kernel/entry-common.S >>> @@ -288,16 +288,15 @@ __sys_trace: >>> cmp scno, #-1 @ skip the syscall? >>> bne 2b >>> add sp, sp, #S_OFF @ restore stack >>> - b ret_slow_syscall >>> >>> -__sys_trace_return: >>> - str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 >>> +__sys_trace_return_nosave: >>> + enable_irq_notrace > > Why is __sys_trace_return_nosave the correct destination here? The > original handle set up for lr a few lines above is for > __sys_trace_return. It's not clear to me why this change is made? __sys_trace_return stores the current r0 value on the stack which will reloaded on exit to user mode. However if skipping a system call r0 is -1 and storing it would destroy the users r0 value, unlike the case where the system call is made and r0 is the return value. The enabling of interrupts is redundant for this purpose, the reuse of code is a size optimization. >>> mov r0, sp >>> bl syscall_trace_exit >>> b ret_slow_syscall >>> >>> -__sys_trace_return_nosave: >>> - enable_irq_notrace >>> +__sys_trace_return: >>> + str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 >>> mov r0, sp >>> bl syscall_trace_exit >>> b ret_slow_syscall ^ permalink raw reply [flat|nested] 13+ messages in thread
* ARM: Call syscall_trace_exit even when system call skipped 2018-03-15 10:38 ` T.E.Baldwin99 at members.leeds.ac.uk @ 2018-03-15 20:14 ` Kees Cook 2018-09-29 23:03 ` Eugene Syromyatnikov 0 siblings, 1 reply; 13+ messages in thread From: Kees Cook @ 2018-03-15 20:14 UTC (permalink / raw) To: linux-arm-kernel On Thu, Mar 15, 2018 at 3:38 AM, <T.E.Baldwin99@members.leeds.ac.uk> wrote: > On 15 March 2018 00:45:01 Kees Cook <keescook@chromium.org> wrote: > >>>> --- a/arch/arm/kernel/entry-common.S >>>> +++ b/arch/arm/kernel/entry-common.S >>>> @@ -288,16 +288,15 @@ __sys_trace: >>>> cmp scno, #-1 @ skip the syscall? >>>> bne 2b >>>> add sp, sp, #S_OFF @ restore stack >>>> - b ret_slow_syscall >>>> >>>> -__sys_trace_return: >>>> - str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 >>>> +__sys_trace_return_nosave: >>>> + enable_irq_notrace >> >> >> Why is __sys_trace_return_nosave the correct destination here? The >> original handle set up for lr a few lines above is for >> __sys_trace_return. It's not clear to me why this change is made? > > > __sys_trace_return stores the current r0 value on the stack which will > reloaded on exit to user mode. However if skipping a system call r0 is -1 > and storing it would destroy the users r0 value, unlike the case where the > system call is made and r0 is the return value. > > The enabling of interrupts is redundant for this purpose, the reuse of code > is a size optimization. Ah right. Cool, thanks! Reviewed-by: Kees Cook <keescook@chromium.org> Tested-by: Kees Cook <keescook@chromium.org> I assume Dmitry can add a Tested-by too? This seems good to go into the ARM patch tracker... -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 13+ messages in thread
* ARM: Call syscall_trace_exit even when system call skipped 2018-03-15 20:14 ` Kees Cook @ 2018-09-29 23:03 ` Eugene Syromyatnikov 2018-10-08 7:11 ` Eugene Syromyatnikov 0 siblings, 1 reply; 13+ messages in thread From: Eugene Syromyatnikov @ 2018-09-29 23:03 UTC (permalink / raw) To: linux-arm-kernel On Thu, Mar 15, 2018 at 01:14:31PM -0700, Kees Cook wrote: > On Thu, Mar 15, 2018 at 3:38 AM, <T.E.Baldwin99@members.leeds.ac.uk> wrote: > > On 15 March 2018 00:45:01 Kees Cook <keescook@chromium.org> wrote: > > > >>>> --- a/arch/arm/kernel/entry-common.S > >>>> +++ b/arch/arm/kernel/entry-common.S > >>>> @@ -288,16 +288,15 @@ __sys_trace: > >>>> cmp scno, #-1 @ skip the syscall? > >>>> bne 2b > >>>> add sp, sp, #S_OFF @ restore stack > >>>> - b ret_slow_syscall > >>>> > >>>> -__sys_trace_return: > >>>> - str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 > >>>> +__sys_trace_return_nosave: > >>>> + enable_irq_notrace > >> > >> > >> Why is __sys_trace_return_nosave the correct destination here? The > >> original handle set up for lr a few lines above is for > >> __sys_trace_return. It's not clear to me why this change is made? > > > > > > __sys_trace_return stores the current r0 value on the stack which will > > reloaded on exit to user mode. However if skipping a system call r0 is -1 > > and storing it would destroy the users r0 value, unlike the case where the > > system call is made and r0 is the return value. > > > > The enabling of interrupts is redundant for this purpose, the reuse of code > > is a size optimization. > > Ah right. Cool, thanks! > > Reviewed-by: Kees Cook <keescook@chromium.org> > Tested-by: Kees Cook <keescook@chromium.org> Tested-by: Eugene Syromyatnikov <evgsyr@gmail.com> Have checked the patch on an Orange Pi Zero board (Allwinner sun8i), kernel 4.14.73-sunxi armv7l (Armbian). Before the change, seccomp_ret_trap test from ldv/SECCOMP_RET_TRAP strace branch was failing with incorrect decoding of the second chdir() call: ---8<--- FAIL: seccomp_ret_trap.gen ========================== --- .././seccomp_ret_trap.expected 2018-09-29 20:16:27.675951746 +0000 +++ log 2018-09-29 21:09:49.669414847 +0000 @@ -1,3 +1,3 @@ chdir(".") = 0 -chdir("./") = 0 +chdir(NULL) = 0 +++ exited with 0 +++ seccomp_ret_trap.gen.test: failed test: ../../strace -a11 -esignal=none -echdir ../seccomp_ret_trap output mismatch FAIL seccomp_ret_trap.gen.test (exit status: 1) --->8--- After the change, test passes and chdir() call after the filtered nanosleep() call is decoded correctly: ---8<--- $ ./strace -echdir,nanosleep,exit_group tests/seccomp_ret_trap chdir(".") = 0 nanosleep({tv_sec=0, tv_nsec=0}, NULL) = 5143188 --- SIGSYS {si_signo=SIGSYS, si_code=SYS_SECCOMP, si_call_addr=0xb6f6df42, si_syscall=__NR_nanosleep, si_arch=AUDIT_ARCH_ARM} --- chdir("./") = 0 exit_group(0) = ? +++ exited with 0 +++ --->8--- > I assume Dmitry can add a Tested-by too? > > This seems good to go into the ARM patch tracker... > > -Kees > > -- > Kees Cook > Pixel Security ^ permalink raw reply [flat|nested] 13+ messages in thread
* ARM: Call syscall_trace_exit even when system call skipped 2018-09-29 23:03 ` Eugene Syromyatnikov @ 2018-10-08 7:11 ` Eugene Syromyatnikov 2018-10-08 9:58 ` Vladimir Murzin 0 siblings, 1 reply; 13+ messages in thread From: Eugene Syromyatnikov @ 2018-10-08 7:11 UTC (permalink / raw) To: linux-arm-kernel On Sun, Sep 30, 2018 at 1:03 AM Eugene Syromyatnikov <evgsyr@gmail.com> wrote: > > On Thu, Mar 15, 2018 at 01:14:31PM -0700, Kees Cook wrote: > > On Thu, Mar 15, 2018 at 3:38 AM, <T.E.Baldwin99@members.leeds.ac.uk> wrote: > > > On 15 March 2018 00:45:01 Kees Cook <keescook@chromium.org> wrote: > > > > > >>>> --- a/arch/arm/kernel/entry-common.S > > >>>> +++ b/arch/arm/kernel/entry-common.S > > >>>> @@ -288,16 +288,15 @@ __sys_trace: > > >>>> cmp scno, #-1 @ skip the syscall? > > >>>> bne 2b > > >>>> add sp, sp, #S_OFF @ restore stack > > >>>> - b ret_slow_syscall > > >>>> > > >>>> -__sys_trace_return: > > >>>> - str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 > > >>>> +__sys_trace_return_nosave: > > >>>> + enable_irq_notrace > > >> > > >> > > >> Why is __sys_trace_return_nosave the correct destination here? The > > >> original handle set up for lr a few lines above is for > > >> __sys_trace_return. It's not clear to me why this change is made? > > > > > > > > > __sys_trace_return stores the current r0 value on the stack which will > > > reloaded on exit to user mode. However if skipping a system call r0 is -1 > > > and storing it would destroy the users r0 value, unlike the case where the > > > system call is made and r0 is the return value. > > > > > > The enabling of interrupts is redundant for this purpose, the reuse of code > > > is a size optimization. > > > > Ah right. Cool, thanks! > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > Tested-by: Kees Cook <keescook@chromium.org> > > Tested-by: Eugene Syromyatnikov <evgsyr@gmail.com> Hello, is anything else required regarding this patch? Thanks. > Have checked the patch on an Orange Pi Zero board (Allwinner sun8i), > kernel 4.14.73-sunxi armv7l (Armbian). > > Before the change, seccomp_ret_trap test from ldv/SECCOMP_RET_TRAP strace branch > was failing with incorrect decoding of the second chdir() call: > > ---8<--- > FAIL: seccomp_ret_trap.gen > ========================== > > --- .././seccomp_ret_trap.expected 2018-09-29 20:16:27.675951746 +0000 > +++ log 2018-09-29 21:09:49.669414847 +0000 > @@ -1,3 +1,3 @@ > chdir(".") = 0 > -chdir("./") = 0 > +chdir(NULL) = 0 > +++ exited with 0 +++ > seccomp_ret_trap.gen.test: failed test: ../../strace -a11 -esignal=none -echdir > ../seccomp_ret_trap output mismatch > FAIL seccomp_ret_trap.gen.test (exit status: 1) > --->8--- > > After the change, test passes and chdir() call after the filtered nanosleep() > call is decoded correctly: > > ---8<--- > $ ./strace -echdir,nanosleep,exit_group tests/seccomp_ret_trap > chdir(".") = 0 > nanosleep({tv_sec=0, tv_nsec=0}, NULL) = 5143188 > --- SIGSYS {si_signo=SIGSYS, si_code=SYS_SECCOMP, si_call_addr=0xb6f6df42, > si_syscall=__NR_nanosleep, si_arch=AUDIT_ARCH_ARM} --- > chdir("./") = 0 > exit_group(0) = ? > +++ exited with 0 +++ > --->8--- > > > I assume Dmitry can add a Tested-by too? > > > > This seems good to go into the ARM patch tracker... > > > > -Kees > > > > -- > > Kees Cook > > Pixel Security -- Eugene Syromyatnikov mailto:evgsyr at gmail.com xmpp:esyr at jabber.{ru|org} ^ permalink raw reply [flat|nested] 13+ messages in thread
* ARM: Call syscall_trace_exit even when system call skipped 2018-10-08 7:11 ` Eugene Syromyatnikov @ 2018-10-08 9:58 ` Vladimir Murzin 0 siblings, 0 replies; 13+ messages in thread From: Vladimir Murzin @ 2018-10-08 9:58 UTC (permalink / raw) To: linux-arm-kernel On 08/10/18 08:11, Eugene Syromyatnikov wrote: > On Sun, Sep 30, 2018 at 1:03 AM Eugene Syromyatnikov <evgsyr@gmail.com> wrote: >> >> On Thu, Mar 15, 2018 at 01:14:31PM -0700, Kees Cook wrote: >>> On Thu, Mar 15, 2018 at 3:38 AM, <T.E.Baldwin99@members.leeds.ac.uk> wrote: >>>> On 15 March 2018 00:45:01 Kees Cook <keescook@chromium.org> wrote: >>>> >>>>>>> --- a/arch/arm/kernel/entry-common.S >>>>>>> +++ b/arch/arm/kernel/entry-common.S >>>>>>> @@ -288,16 +288,15 @@ __sys_trace: >>>>>>> cmp scno, #-1 @ skip the syscall? >>>>>>> bne 2b >>>>>>> add sp, sp, #S_OFF @ restore stack >>>>>>> - b ret_slow_syscall >>>>>>> >>>>>>> -__sys_trace_return: >>>>>>> - str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 >>>>>>> +__sys_trace_return_nosave: >>>>>>> + enable_irq_notrace >>>>> >>>>> >>>>> Why is __sys_trace_return_nosave the correct destination here? The >>>>> original handle set up for lr a few lines above is for >>>>> __sys_trace_return. It's not clear to me why this change is made? >>>> >>>> >>>> __sys_trace_return stores the current r0 value on the stack which will >>>> reloaded on exit to user mode. However if skipping a system call r0 is -1 >>>> and storing it would destroy the users r0 value, unlike the case where the >>>> system call is made and r0 is the return value. >>>> >>>> The enabling of interrupts is redundant for this purpose, the reuse of code >>>> is a size optimization. >>> >>> Ah right. Cool, thanks! >>> >>> Reviewed-by: Kees Cook <keescook@chromium.org> >>> Tested-by: Kees Cook <keescook@chromium.org> >> >> Tested-by: Eugene Syromyatnikov <evgsyr@gmail.com> > > Hello, is anything else required regarding this patch? Thanks. I believe you need to drop it into Russell's patch tracker [1]. [1] http://www.armlinux.org.uk/developer/patches/ Cheers Vladimir ^ permalink raw reply [flat|nested] 13+ messages in thread
* ARM: Call syscall_trace_exit even when system call skipped 2018-02-03 15:21 [PATCH] ARM: Call syscall_trace_exit even when system call skipped Timothy E Baldwin 2018-03-13 23:58 ` Dmitry V. Levin @ 2018-10-08 11:00 ` Eugene Syromyatnikov 2018-10-08 18:08 ` Kees Cook 2018-10-08 18:26 ` Eugene Syromyatnikov 2 siblings, 1 reply; 13+ messages in thread From: Eugene Syromyatnikov @ 2018-10-08 11:00 UTC (permalink / raw) To: linux-arm-kernel From: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk> On at least x86 and ARM64, and as documented in the ptrace man page a skipped system call will still cause a syscall exit ptrace stop. Previous to this commit 32-bit ARM did not, resulting in strace being confused when seccomp skips system calls. This change also impacts programs that use ptrace to skip system calls. Fixes: ad75b51459ae ("ARM: 7579/1: arch/allow a scno of -1 to not cause a SIGILL") Signed-off-by: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk> Signed-off-by: Eugene Syromyatnikov <evgsyr@gmail.com> Reviewed-by: Kees Cook <keescook@chromium.org> Tested-by: Kees Cook <keescook@chromium.org> Tested-by: Eugene Syromyatnikov <evgsyr@gmail.com> --- KernelVersion: 4.19-rc7 diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S index 746565a..0465d65 100644 --- a/arch/arm/kernel/entry-common.S +++ b/arch/arm/kernel/entry-common.S @@ -296,16 +296,15 @@ __sys_trace: cmp scno, #-1 @ skip the syscall? bne 2b add sp, sp, #S_OFF @ restore stack - b ret_slow_syscall -__sys_trace_return: - str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 +__sys_trace_return_nosave: + enable_irq_notrace mov r0, sp bl syscall_trace_exit b ret_slow_syscall -__sys_trace_return_nosave: - enable_irq_notrace +__sys_trace_return: + str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 mov r0, sp bl syscall_trace_exit b ret_slow_syscall -- 2.1.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* ARM: Call syscall_trace_exit even when system call skipped 2018-10-08 11:00 ` Eugene Syromyatnikov @ 2018-10-08 18:08 ` Kees Cook 2018-10-08 18:13 ` Russell King - ARM Linux 0 siblings, 1 reply; 13+ messages in thread From: Kees Cook @ 2018-10-08 18:08 UTC (permalink / raw) To: linux-arm-kernel On Mon, Oct 8, 2018 at 4:00 AM, Eugene Syromyatnikov <evgsyr@gmail.com> wrote: > From: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk> > > On at least x86 and ARM64, and as documented in the ptrace man page > a skipped system call will still cause a syscall exit ptrace stop. > > Previous to this commit 32-bit ARM did not, resulting in strace > being confused when seccomp skips system calls. > > This change also impacts programs that use ptrace to skip system calls. > > Fixes: ad75b51459ae ("ARM: 7579/1: arch/allow a scno of -1 to not cause a SIGILL") > Signed-off-by: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk> > Signed-off-by: Eugene Syromyatnikov <evgsyr@gmail.com> > Reviewed-by: Kees Cook <keescook@chromium.org> > Tested-by: Kees Cook <keescook@chromium.org> > Tested-by: Eugene Syromyatnikov <evgsyr@gmail.com> > --- > KernelVersion: 4.19-rc7 Did this actually make it into the patch tracker? I don't see it in Incoming... -Kees > diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S > index 746565a..0465d65 100644 > --- a/arch/arm/kernel/entry-common.S > +++ b/arch/arm/kernel/entry-common.S > @@ -296,16 +296,15 @@ __sys_trace: > cmp scno, #-1 @ skip the syscall? > bne 2b > add sp, sp, #S_OFF @ restore stack > - b ret_slow_syscall > > -__sys_trace_return: > - str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 > +__sys_trace_return_nosave: > + enable_irq_notrace > mov r0, sp > bl syscall_trace_exit > b ret_slow_syscall > > -__sys_trace_return_nosave: > - enable_irq_notrace > +__sys_trace_return: > + str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 > mov r0, sp > bl syscall_trace_exit > b ret_slow_syscall > -- > 2.1.4 > -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 13+ messages in thread
* ARM: Call syscall_trace_exit even when system call skipped 2018-10-08 18:08 ` Kees Cook @ 2018-10-08 18:13 ` Russell King - ARM Linux 2018-10-08 18:33 ` Eugene Syromyatnikov 0 siblings, 1 reply; 13+ messages in thread From: Russell King - ARM Linux @ 2018-10-08 18:13 UTC (permalink / raw) To: linux-arm-kernel On Mon, Oct 08, 2018 at 11:08:16AM -0700, Kees Cook wrote: > On Mon, Oct 8, 2018 at 4:00 AM, Eugene Syromyatnikov <evgsyr@gmail.com> wrote: > > From: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk> > > > > On at least x86 and ARM64, and as documented in the ptrace man page > > a skipped system call will still cause a syscall exit ptrace stop. > > > > Previous to this commit 32-bit ARM did not, resulting in strace > > being confused when seccomp skips system calls. > > > > This change also impacts programs that use ptrace to skip system calls. > > > > Fixes: ad75b51459ae ("ARM: 7579/1: arch/allow a scno of -1 to not cause a SIGILL") > > Signed-off-by: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk> > > Signed-off-by: Eugene Syromyatnikov <evgsyr@gmail.com> > > Reviewed-by: Kees Cook <keescook@chromium.org> > > Tested-by: Kees Cook <keescook@chromium.org> > > Tested-by: Eugene Syromyatnikov <evgsyr@gmail.com> > > --- > > KernelVersion: 4.19-rc7 > > Did this actually make it into the patch tracker? I don't see it in Incoming... No, rejected at SMTP time (so bounced) due to it being sent from a gmail.com address but _not_ going through gmail servers (so no DKIM signature.) Hence, looks like all the other fake gmail crud and gets rejected outright. I guess if Eugene didn't get the bounce notification, then gmail decided to discard that as fake too! -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ^ permalink raw reply [flat|nested] 13+ messages in thread
* ARM: Call syscall_trace_exit even when system call skipped 2018-10-08 18:13 ` Russell King - ARM Linux @ 2018-10-08 18:33 ` Eugene Syromyatnikov 0 siblings, 0 replies; 13+ messages in thread From: Eugene Syromyatnikov @ 2018-10-08 18:33 UTC (permalink / raw) To: linux-arm-kernel On Mon, Oct 08, 2018 at 07:13:42PM +0100, Russell King - ARM Linux wrote: > On Mon, Oct 08, 2018 at 11:08:16AM -0700, Kees Cook wrote: > > Did this actually make it into the patch tracker? I don't see it in Incoming... > > No, rejected at SMTP time (so bounced) due to it being sent from a > gmail.com address but _not_ going through gmail servers (so no DKIM > signature.) Hence, looks like all the other fake gmail crud and gets > rejected outright. > > I guess if Eugene didn't get the bounce notification, then gmail > decided to discard that as fake too! My apologies, I've mixed up two systems with different mail client setups. ^ permalink raw reply [flat|nested] 13+ messages in thread
* ARM: Call syscall_trace_exit even when system call skipped 2018-02-03 15:21 [PATCH] ARM: Call syscall_trace_exit even when system call skipped Timothy E Baldwin 2018-03-13 23:58 ` Dmitry V. Levin 2018-10-08 11:00 ` Eugene Syromyatnikov @ 2018-10-08 18:26 ` Eugene Syromyatnikov 2 siblings, 0 replies; 13+ messages in thread From: Eugene Syromyatnikov @ 2018-10-08 18:26 UTC (permalink / raw) To: linux-arm-kernel From: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk> On at least x86 and ARM64, and as documented in the ptrace man page a skipped system call will still cause a syscall exit ptrace stop. Previous to this commit 32-bit ARM did not, resulting in strace being confused when seccomp skips system calls. This change also impacts programs that use ptrace to skip system calls. Fixes: ad75b51459ae ("ARM: 7579/1: arch/allow a scno of -1 to not cause a SIGILL") Signed-off-by: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk> Signed-off-by: Eugene Syromyatnikov <evgsyr@gmail.com> Reviewed-by: Kees Cook <keescook@chromium.org> Tested-by: Kees Cook <keescook@chromium.org> Tested-by: Eugene Syromyatnikov <evgsyr@gmail.com> --- KernelVersion: 4.19-rc7 diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S index 746565a..0465d65 100644 --- a/arch/arm/kernel/entry-common.S +++ b/arch/arm/kernel/entry-common.S @@ -296,16 +296,15 @@ __sys_trace: cmp scno, #-1 @ skip the syscall? bne 2b add sp, sp, #S_OFF @ restore stack - b ret_slow_syscall -__sys_trace_return: - str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 +__sys_trace_return_nosave: + enable_irq_notrace mov r0, sp bl syscall_trace_exit b ret_slow_syscall -__sys_trace_return_nosave: - enable_irq_notrace +__sys_trace_return: + str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 mov r0, sp bl syscall_trace_exit b ret_slow_syscall -- 2.1.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-10-08 18:33 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-03 15:21 [PATCH] ARM: Call syscall_trace_exit even when system call skipped Timothy E Baldwin 2018-03-13 23:58 ` Dmitry V. Levin 2018-03-15 0:44 ` Kees Cook 2018-03-15 10:38 ` T.E.Baldwin99 at members.leeds.ac.uk 2018-03-15 20:14 ` Kees Cook 2018-09-29 23:03 ` Eugene Syromyatnikov 2018-10-08 7:11 ` Eugene Syromyatnikov 2018-10-08 9:58 ` Vladimir Murzin 2018-10-08 11:00 ` Eugene Syromyatnikov 2018-10-08 18:08 ` Kees Cook 2018-10-08 18:13 ` Russell King - ARM Linux 2018-10-08 18:33 ` Eugene Syromyatnikov 2018-10-08 18:26 ` Eugene Syromyatnikov
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).