public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Raoni Fassina Firmino <raoni@linux.ibm.com>
To: libc-alpha@sourceware.org
Cc: Florian Weimer <fweimer@redhat.com>,
	Adhemerval Zanella <adhemerval.zanella@linaro.org>
Subject: [PATCH v2] powerpc64: Workaround sigtramp vdso return call
Date: Wed, 27 Jan 2021 16:23:05 -0300	[thread overview]
Message-ID: <20210127192305.pw74xougw2ejihuz@work-tp> (raw)

Changes since v1[1]:
  - Fixed comments length and formatting;
  - Changed comment wording to the one suggested by Adhermeval;
  - Changed if logic to the one suggested by Adhermeval.

Original message:
  There was some initial discussions on the mailing list about the
  failing misc/tst-sigcontext-get_pc[0], and I made some suggestions
  of possible solutions.  As the window for the release is closing I
  want to sent the more simple one of them ASAP for consideration
  (others would not make it in time)

Tested on top of master (01cdcf783a666481133d4975b1980624b0ef4799)
on the following platforms with no regression:
  - powerpc64le-linux-gnu (Power 9) kernel 5.4.0-59
  - powerpc64le-linux-gnu (Power 9) kernel 5.9.14-1
  - powerpc64-linux-gnu (Power 9) kernel 5.10.0-1

[0] https://sourceware.org/pipermail/libc-alpha/2021-January/122027.html
[1] https://sourceware.org/pipermail/libc-alpha/2021-January/121933.html

---- 8< ----

A not so recent kernel change[1] changed how the trampoline
`__kernel_sigtramp_rt64` is used to call signal handlers.

This was exposed on the test misc/tst-sigcontext-get_pc

Before kernel 5.9, the kernel set LR to the trampoline address and
jumped directly to the signal handler, and at the end the signal
handler, as any other function, would `blr` to the address set.  In
other words, the trampoline was executed just at the end of the signal
handler and the only thing it did was call sigreturn.  But since
kernel 5.9 the kernel set CTRL to the signal handler and calls to the
trampoline code, the trampoline then `bctrl` to the address in CTRL,
setting the LR to the next instruction in the middle of the
trampoline, when the signal handler returns, the rest of the
trampoline code executes the same code as before.

Here is the full trampoline code as of kernel 5.11.0-rc5 for
reference:

    V_FUNCTION_BEGIN(__kernel_sigtramp_rt64)
    .Lsigrt_start:
            bctrl▸  /* call the handler */
            addi▸   r1, r1, __SIGNAL_FRAMESIZE
            li▸     r0,__NR_rt_sigreturn
            sc
    .Lsigrt_end:
    V_FUNCTION_END(__kernel_sigtramp_rt64)

This new behavior breaks how `backtrace()` uses to detect the
trampoline frame to correctly reconstruct the stack frame when it is
called from inside a signal handling.

This workaround rely on the fact that the trampoline code is at very
least two (maybe 3?) instructions in size (as it is in the 32 bits
version, only on `li` and `sc`), so it is safe to check the return
address be in the range __kernel_sigtramp_rt64 .. + 4.

[1] subject: powerpc/64/signal: Balance return predictor stack in signal trampoline
    commit: 0138ba5783ae0dcc799ad401a1e8ac8333790df9
    url: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0138ba5783ae0dcc799ad401a1e8ac8333790df9
---
 sysdeps/powerpc/powerpc64/backtrace.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/sysdeps/powerpc/powerpc64/backtrace.c b/sysdeps/powerpc/powerpc64/backtrace.c
index ae64c5d7a5..37de9b5bdd 100644
--- a/sysdeps/powerpc/powerpc64/backtrace.c
+++ b/sysdeps/powerpc/powerpc64/backtrace.c
@@ -54,11 +54,22 @@ struct signal_frame_64 {
   /* We don't care about the rest, since the IP value is at 'uc' field.  */
 };
 
+/* Test if the address match to the inside the trampoline code.
+   Up to and including kernel 5.8, returning from an interrupt or syscall to a
+   signal handler starts execution directly at the handler's entry point, with
+   LR set to address of the sigreturn trampoline (the vDSO symbol).
+   Newer kernels will branch to signal handler from the trampoline instead, so
+   checking the stacktrace against the vDSO entrypoint does not work in such
+   case.
+   The vDSO branches with a 'bctrl' instruction, so checking either the
+   vDSO address itself and the next instruction should cover all kernel
+   versions.  */
 static inline bool
 is_sigtramp_address (void *nip)
 {
 #ifdef HAVE_SIGTRAMP_RT64
-  if (nip == GLRO (dl_vdso_sigtramp_rt64))
+  if (nip == GLRO (dl_vdso_sigtramp_rt64) ||
+      nip == GLRO (dl_vdso_sigtramp_rt64) + 4)
     return true;
 #endif
   return false;
-- 
2.26.2


             reply	other threads:[~2021-01-27 19:23 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-27 19:23 Raoni Fassina Firmino [this message]
2021-01-27 19:33 ` Adhemerval Zanella
2021-01-28 16:59   ` Tulio Magno Quites Machado Filho
2021-02-11 21:55     ` Florian Weimer
2021-02-12  2:14       ` Adhemerval Zanella
2021-02-15 17:51       ` Raoni Fassina Firmino

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210127192305.pw74xougw2ejihuz@work-tp \
    --to=raoni@linux.ibm.com \
    --cc=adhemerval.zanella@linaro.org \
    --cc=fweimer@redhat.com \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).