[PATCH] ARM: Call syscall_trace_exit even when system call skipped

[PATCH] ARM: Call syscall_trace_exit even when system call skipped

[Timothy E Baldwin]

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

ARM: Call syscall_trace_exit even when system call skipped

[Dmitry V. Levin]

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>

ARM: Call syscall_trace_exit even when system call skipped

[Kees Cook]

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

ARM: Call syscall_trace_exit even when system call skipped

[T.E.Baldwin99 at members.leeds.ac.uk]

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

ARM: Call syscall_trace_exit even when system call skipped

[Kees Cook]

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

ARM: Call syscall_trace_exit even when system call skipped

[Eugene Syromyatnikov]

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

ARM: Call syscall_trace_exit even when system call skipped

[Eugene Syromyatnikov]

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}

ARM: Call syscall_trace_exit even when system call skipped

[Vladimir Murzin]

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

ARM: Call syscall_trace_exit even when system call skipped

[Eugene Syromyatnikov]

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

ARM: Call syscall_trace_exit even when system call skipped

[Kees Cook]

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

ARM: Call syscall_trace_exit even when system call skipped

[Russell King - ARM Linux]

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

ARM: Call syscall_trace_exit even when system call skipped

[Eugene Syromyatnikov]

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

ARM: Call syscall_trace_exit even when system call skipped

[Eugene Syromyatnikov]

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.