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

Vineet Gupta
2020-11-06T20:27:54Z

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@uclibc-ng.org>
> Bisected-by: Waldemar Brodkorb <wbx@uclibc-ng.org>
> Cc: stable <stable@vger.kernel.org> # 5.2 and prior
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>

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
>