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

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

[Raoni Fassina Firmino]

Repeated the same tests as the upstream code on top of v5.10.14 and
v5.9.16, 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. I hope I had tagged this
correctly in the patch.

The commit message bellow is cherry-picked from the upstream commit, I
am not sure what should I do with the footer, I left it as-is and just
added a [rff: Backported] at the end.

---- 8< ----

commit 24321ac668e452a4942598533d267805f291fdc9 upstream.

This backport differ from the upstream patch in the way to set the
sigtramp offsets, after 5.10 VDSO symbols offsets are retrieved at
buildtime and before, in this patch it uses the runtime generated
offsets logic.

Commit 0138ba5783ae ("powerpc/64/signal: Balance return predictor
stack in signal trampoline") changed __kernel_sigtramp_rt64() VDSO and
trampoline code, and introduced a regression in the way glibc's
backtrace()[1] detects the signal-handler stack frame. Apart from the
practical implications, __kernel_sigtramp_rt64() was a VDSO function
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 behaviour of 0138ba5783ae by adding a new symbol
to serve as the jump target from the kernel to the trampoline. Now the
trampoline has two parts, a new entry point and the old return point.

[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-January/223194.html

Fixes: 0138ba5783ae ("powerpc/64/signal: Balance return predictor stack in signal trampoline")
Cc: stable@vger.kernel.org # v5.9+
Signed-off-by: Raoni Fassina Firmino <raoni@linux.ibm.com>
Acked-by: Nicholas Piggin <npiggin@gmail.com>
[mpe: Minor tweaks to change log formatting, add stable tag]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20210201200505.iz46ubcizipnkcxe@work-tp
[rff: Backported]
---
 arch/powerpc/kernel/vdso.c              |  2 +-
 arch/powerpc/kernel/vdso64/sigtramp.S   | 11 ++++++++++-
 arch/powerpc/kernel/vdso64/vdso64.lds.S |  1 +
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index 8dad44262e75..495ffc9cf5e2 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -475,7 +475,7 @@ static __init void vdso_setup_trampolines(struct lib32_elfinfo *v32,
 	 */
 
 #ifdef CONFIG_PPC64
-	vdso64_rt_sigtramp = find_function64(v64, "__kernel_sigtramp_rt64");
+	vdso64_rt_sigtramp = find_function64(v64, "__kernel_start_sigtramp_rt64");
 #endif
 	vdso32_sigtramp	   = find_function32(v32, "__kernel_sigtramp32");
 	vdso32_rt_sigtramp = find_function32(v32, "__kernel_sigtramp_rt32");
diff --git a/arch/powerpc/kernel/vdso64/sigtramp.S b/arch/powerpc/kernel/vdso64/sigtramp.S
index bbf68cd01088..2d4067561293 100644
--- a/arch/powerpc/kernel/vdso64/sigtramp.S
+++ b/arch/powerpc/kernel/vdso64/sigtramp.S
@@ -15,11 +15,20 @@
 
 	.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.
+ */
 	.balign 8
 	.balign IFETCH_ALIGN_BYTES
-V_FUNCTION_BEGIN(__kernel_sigtramp_rt64)
+V_FUNCTION_BEGIN(__kernel_start_sigtramp_rt64)
 .Lsigrt_start:
 	bctrl	/* call the handler */
+V_FUNCTION_END(__kernel_start_sigtramp_rt64)
+V_FUNCTION_BEGIN(__kernel_sigtramp_rt64)
 	addi	r1, r1, __SIGNAL_FRAMESIZE
 	li	r0,__NR_rt_sigreturn
 	sc
diff --git a/arch/powerpc/kernel/vdso64/vdso64.lds.S b/arch/powerpc/kernel/vdso64/vdso64.lds.S
index 256fb9720298..bd120f590b9e 100644
--- a/arch/powerpc/kernel/vdso64/vdso64.lds.S
+++ b/arch/powerpc/kernel/vdso64/vdso64.lds.S
@@ -150,6 +150,7 @@ VERSION
 		__kernel_get_tbfreq;
 		__kernel_sync_dicache;
 		__kernel_sync_dicache_p5;
+		__kernel_start_sigtramp_rt64;
 		__kernel_sigtramp_rt64;
 		__kernel_getcpu;
 		__kernel_time;

base-commit: b0c8835fc649454c33371f4617111cb5d60463e1
-- 
2.26.2

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

[Greg KH]

On Tue, Feb 09, 2021 at 12:02:40PM -0300, Raoni Fassina Firmino wrote:
> Repeated the same tests as the upstream code on top of v5.10.14 and
> v5.9.16, 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. I hope I had tagged this
> correctly in the patch.

Now added to 5.10.y, 5.9.y is long end-of-life so there is nothing we
can do there, sorry.

thanks for the backport,

greg k-h

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

[Raoni Fassina Firmino]

On Wed, Feb 10, 2021 at 03:27:05PM +0100, Greg KH wrote:
> On Tue, Feb 09, 2021 at 12:02:40PM -0300, Raoni Fassina Firmino wrote:
> > Repeated the same tests as the upstream code on top of v5.10.14 and
> > v5.9.16, 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. I hope I had tagged this
> > correctly in the patch.
> 
> Now added to 5.10.y, 5.9.y is long end-of-life so there is nothing we
> can do there, sorry.

No problem, I didn't know 5.9.y was already EOL, that is on me.

Thanks,

o/
Raoni

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

[Greg KH]

On Thu, Feb 11, 2021 at 08:28:09AM -0300, Raoni Fassina Firmino wrote:
> On Wed, Feb 10, 2021 at 03:27:05PM +0100, Greg KH wrote:
> > On Tue, Feb 09, 2021 at 12:02:40PM -0300, Raoni Fassina Firmino wrote:
> > > Repeated the same tests as the upstream code on top of v5.10.14 and
> > > v5.9.16, 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. I hope I had tagged this
> > > correctly in the patch.
> > 
> > Now added to 5.10.y, 5.9.y is long end-of-life so there is nothing we
> > can do there, sorry.
> 
> No problem, I didn't know 5.9.y was already EOL, that is on me.

Hint, in the future www.kernel.org shows you this :)