ARM: Call syscall_trace_exit even when system call skipped

[vladimir.murzin@arm.com (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