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< ----
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(-)
From: Nicholas Piggin <npiggin@gmail.com> Date: 2021-02-03 00:10:16
Excerpts from Raoni Fassina Firmino's message of February 2, 2021 6:05 am:
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.
Thanks,
Nick
quoted hunk
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(-)
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! :)
Thanks,
Nick
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2021-02-03 01:19:59
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(-)
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
On Tue, Feb 02, 2021 at 05:41:35PM +1000, Nicholas Piggin wrote:
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).
For glibc from the beginning I planned to send a patch as well and
fortunately it got in[1] in time for yesterday's 2.33 release. That
patch is meant to address kernels 5.9 and 5.10 but is also compatible
with older kernels and with this patch. So everyone should be compatible
with everyone else :-) (except unpatched kernels 5.9 and 5.10 with
unpatched glibcs prior to 2.33)
I don't know about musl, I took a look and maybe wrong here but didn't
find any backtrace() implementation and it seems that it uses its own
return code for the trampoline, but I am not sure.
Rich mentioned[2] that he didn't see how it would break musl and was
waiting for feedback from ppc users to be sure.
Thanks for the review :-)
o/
Raoni
[1] https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=5ee506ed35a2c9184bcb1fb5e79b6cceb9bb0dd1
[2] https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-January/223198.html
From: Michael Ellerman <hidden> Date: 2021-02-03 13:43:22
On Mon, 1 Feb 2021 17:05:05 -0300, Raoni Fassina Firmino wrote:
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.
[...]