ARM: Call syscall_trace_exit even when system call skipped

On 08/10/18 08:11, Eugene Syromyatnikov wrote:

On Sun, Sep 30, 2018 at 1:03 AM Eugene Syromyatnikov [off-list ref] wrote:

quoted

On Thu, Mar 15, 2018 at 01:14:31PM -0700, Kees Cook wrote:

quoted

On Thu, Mar 15, 2018 at 3:38 AM,  [off-list ref] wrote:

quoted

On 15 March 2018 00:45:01 Kees Cook [off-list ref] wrote:

quoted

quoted

quoted

--- 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 <redacted>
Tested-by: Kees Cook <redacted>

Tested-by: Eugene Syromyatnikov <redacted>

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