Re: [PATCH] powerpc/64/signal: Fix regression in __kernel_sigtramp_rt64 semantics

Nicholas Piggin [off-list ref] writes:

Excerpts from Raoni Fassina Firmino's message of February 2, 2021 6:05 am:

quoted

Tested on powerpc64 and powerpc64le, with a glibc build and running the
affected glibc's testcase[2], inspected that glibc's backtrace() now gives
the correct result and gdb backtrace also keeps working as before.

I believe this should be backported to releases 5.9 and 5.10 as userspace
is affected in this releases.

---- 8< ----

Thanks for this, I don't know the glibc code but the kernel change seems 
okay to me.

I turned this into an Acked-by from you.
 

quoted

A Change[1] in __kernel_sigtramp_rt64 VDSO and trampoline code introduced a
regression in the way glibc's backtrace()[2] detects the signal-handler
stack frame.  Apart from the practical implications, __kernel_sigtram_rt64
was a VDSO with the semantics that it is a function you can call from
userspace to end a signal handling.  Now this semantics are no longer
valid.

I believe the aforementioned change affects all releases since 5.9.

This patch tries to fix both the semantics and practical aspect of
__kernel_sigtramp_rt64 returning it to the previous code, whilst keeping
the intended behavior from[1] by adding a new symbol to serve as the jump
target from the kernel to the trampoline. Now the trampoline has two parts,
an new entry point and the old return point.

[1] commit 0138ba5783ae0dcc799ad401a1e8ac8333790df9 ("powerpc/64/signal:
    Balance return predictor stack in signal trampoline")
[2] https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-January/223194.html

Fixes: 0138ba5783ae ("powerpc/64/signal: Balance return predictor stack in signal trampoline")
Signed-off-by: Raoni Fassina Firmino <redacted>
---
 arch/powerpc/kernel/vdso64/sigtramp.S   | 9 ++++++++-
 arch/powerpc/kernel/vdso64/vdso64.lds.S | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/vdso64/sigtramp.S b/arch/powerpc/kernel/vdso64/sigtramp.S
index bbf68cd01088..f0fd8d2a9fc4 100644
--- a/arch/powerpc/kernel/vdso64/sigtramp.S
+++ b/arch/powerpc/kernel/vdso64/sigtramp.S

@@ -15,11 +15,18 @@
 
 	.text
 
+/* __kernel_start_sigtramp_rt64 and __kernel_sigtramp_rt64 together
+   are one function split in two parts. The kernel jumps to the former
+   and the signal handler indirectly (by blr) returns to the latter.
+   __kernel_sigtramp_rt64 needs to point to the return address so
+   glibc can correctly identify the trampoline stack frame.  */

Are you planning to update glibc to cope with this as well? Any idea 
about musl? If so, including version numbers would be good (not that
it's really a problem to carry this patch around).

I was just about to ask to turn the comment into kernel style, but the
whole file has this style so nevermind about that! :)

Yeah, copying the existing style was the right thing to do.

... but I really can't deal with that comment style so I reformatted it
to match kernel style :)

Parts of that file use two-space indents as well, the whole thing could
do with a pass through clang-format or similar one day.

cheers