Re: [PATCH] Revert "ARC: entry: fix potential EFA clobber when TIF_SYSCALL_TRACE"

[Vineet Gupta]

Hi Stable Team,

On 10/19/20 7:19 PM, Vineet Gupta wrote:
> This reverts commit 00fdec98d9881bf5173af09aebd353ab3b9ac729.
> (but only from 5.2 and prior kernels)
> 
> The original commit was a preventive fix based on code-review and was
> auto-picked for stable back-port (for better or worse).
> It was OK for v5.3+ kernels, but turned up needing an implicit change
> 68e5c6f073bcf70 "(ARC: entry: EV_Trap expects r10 (vs. r9) to have
>  exception cause)" merged in v5.3 which itself was not backported.
> So to summarize the stable backport of this patch for v5.2 and prior
> kernels is busted and it won't boot.
> 
> The obvious solution is backport 68e5c6f073bcf70 but that is a pain as
> it doesn't revert cleanly and each of affected kernels (so far v4.19,
> v4.14, v4.9, v4.4) needs a slightly different massaged varaint.
> So the easier fix is to simply revert the backport from 5.2 and prior.
> The issue was not a big deal as it would cause strace to sporadically
> not work correctly.
> 
> Waldemar Brodkorb first reported this when running ARC uClibc regressions
> on latest stable kernels (with offending backport). Once he bisected it,
> the analysis was trivial, so thx to him for this.
> 
> Reported-by: Waldemar Brodkorb <wbx@xxxxxxxxxxxxx>
> Bisected-by: Waldemar Brodkorb <wbx@xxxxxxxxxxxxx>
> Cc: stable <stable@xxxxxxxxxxxxxxx> # 5.2 and prior
> Signed-off-by: Vineet Gupta <vgupta@xxxxxxxxxxxx>

Can this revert be please applied to 4.19 and older kernels for the next cycle.

Or is there is a procedural issue given this revert is not in mainline. I've
described the issue in detail above so if there's a better/desirable way of
reverting it from backports, please let me know.

Thx,

> ---
>  arch/arc/kernel/entry.S | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arc/kernel/entry.S b/arch/arc/kernel/entry.S
> index ea00c8a17f07..60406ec62eb8 100644
> --- a/arch/arc/kernel/entry.S
> +++ b/arch/arc/kernel/entry.S
> @@ -165,6 +165,7 @@ END(EV_Extension)
>  tracesys:
>  	; save EFA in case tracer wants the PC of traced task
>  	; using ERET won't work since next-PC has already committed
> +	lr  r12, [efa]
>  	GET_CURR_TASK_FIELD_PTR   TASK_THREAD, r11
>  	st  r12, [r11, THREAD_FAULT_ADDR]	; thread.fault_address
>  
> @@ -207,9 +208,15 @@ tracesys_exit:
>  ; Breakpoint TRAP
>  ; ---------------------------------------------
>  trap_with_param:
> -	mov r0, r12	; EFA in case ptracer/gdb wants stop_pc
> +
> +	; stop_pc info by gdb needs this info
> +	lr  r0, [efa]
>  	mov r1, sp
>  
> +	; Now that we have read EFA, it is safe to do "fake" rtie
> +	;   and get out of CPU exception mode
> +	FAKE_RET_FROM_EXCPN
> +
>  	; Save callee regs in case gdb wants to have a look
>  	; SP will grow up by size of CALLEE Reg-File
>  	; NOTE: clobbers r12
> @@ -236,10 +243,6 @@ ENTRY(EV_Trap)
>  
>  	EXCEPTION_PROLOGUE
>  
> -	lr  r12, [efa]
> -
> -	FAKE_RET_FROM_EXCPN
> -
>  	;============ TRAP 1   :breakpoints
>  	; Check ECR for trap with arg (PROLOGUE ensures r10 has ECR)
>  	bmsk.f 0, r10, 7
> @@ -247,6 +250,9 @@ ENTRY(EV_Trap)
>  
>  	;============ TRAP  (no param): syscall top level
>  
> +	; First return from Exception to pure K mode (Exception/IRQs renabled)
> +	FAKE_RET_FROM_EXCPN
> +
>  	; If syscall tracing ongoing, invoke pre-post-hooks
>  	GET_CURR_THR_INFO_FLAGS   r10
>  	btst r10, TIF_SYSCALL_TRACE
>