public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed
* [glibc] hurd: Fix x86_64 sigreturn restoring bogus reply_port
@ 2023-06-04 17:06 Samuel Thibault
  0 siblings, 0 replies; only message in thread
From: Samuel Thibault @ 2023-06-04 17:06 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=67f704ab69e2305c2b9043d3451df33edbf99b05

commit 67f704ab69e2305c2b9043d3451df33edbf99b05
Author: Sergey Bugaev <bugaevc@gmail.com>
Date:   Sun Jun 4 19:05:51 2023 +0200

    hurd: Fix x86_64 sigreturn restoring bogus reply_port
    
    Since the area of the user's stack we use for the registers dump (and
    otherwise as __sigreturn2's stack) can and does overlap the sigcontext,
    we have to be very careful about the order of loads and stores that we
    do. In particular we have to load sc_reply_port before we start
    clobbering the sigcontext.
    
    Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>

Diff:
---
 sysdeps/mach/hurd/x86_64/sigreturn.c | 84 ++++++++++++++++++++----------------
 1 file changed, 46 insertions(+), 38 deletions(-)

diff --git a/sysdeps/mach/hurd/x86_64/sigreturn.c b/sysdeps/mach/hurd/x86_64/sigreturn.c
index f37ae119ca..94a379d382 100644
--- a/sysdeps/mach/hurd/x86_64/sigreturn.c
+++ b/sysdeps/mach/hurd/x86_64/sigreturn.c
@@ -24,7 +24,7 @@
    unlock SS off sigstack.  */
 void
 __sigreturn2 (struct hurd_sigstate *ss, uintptr_t *usp,
-              struct sigcontext *scp)
+              mach_port_t sc_reply_port)
 {
   mach_port_t reply_port;
   _hurd_sigstate_unlock (ss);
@@ -44,7 +44,7 @@ __sigreturn2 (struct hurd_sigstate *ss, uintptr_t *usp,
   if (__glibc_likely (MACH_PORT_VALID (reply_port)))
     (void) __mach_port_mod_refs (__mach_task_self (), reply_port,
                                  MACH_PORT_RIGHT_RECEIVE, -1);
-  THREAD_SETMEM (THREAD_SELF, reply_port, scp->sc_reply_port);
+  THREAD_SETMEM (THREAD_SELF, reply_port, sc_reply_port);
 
   asm volatile (
                 /* Point the stack to the register dump.  */
@@ -77,6 +77,8 @@ __sigreturn (struct sigcontext *scp)
 {
   struct hurd_sigstate *ss;
   struct hurd_userlink *link = (void *) &scp[1];
+  uintptr_t *usp;
+  mach_port_t sc_reply_port;
 
   if (__glibc_unlikely (scp == NULL || (scp->sc_mask & _SIG_CANT_MASK)))
     return __hurd_fail (EINVAL);
@@ -118,42 +120,48 @@ __sigreturn (struct sigcontext *scp)
        in the format the i387 `frstor' instruction uses to restore it.  */
     asm volatile ("frstor %0" : : "m" (scp->sc_fpsave));
 
-  {
-    /* There are convenient instructions to pop state off the stack, so we
-       copy the registers onto the user's stack, switch there, pop and
-       return.  */
-
-    uintptr_t *usp = (uintptr_t *) (scp->sc_ursp - 128);
-
-    *--usp = scp->sc_rip;
-    *--usp = scp->sc_rfl;
-    *--usp = scp->sc_rax;
-    *--usp = scp->sc_rcx;
-    *--usp = scp->sc_rdx;
-    *--usp = scp->sc_rbx;
-    *--usp = scp->sc_rbp;
-    *--usp = scp->sc_rsi;
-    *--usp = scp->sc_rdi;
-    *--usp = scp->sc_r15;
-    *--usp = scp->sc_r14;
-    *--usp = scp->sc_r13;
-    *--usp = scp->sc_r12;
-    *--usp = scp->sc_r11;
-    *--usp = scp->sc_r10;
-    *--usp = scp->sc_r9;
-    *--usp = scp->sc_r8;
-
-    /* Switch to the user's stack that we have just prepared, and call
-       __sigreturn2.  Clobber "memory" to make sure GCC flushes the stack
-       setup to actual memory.  We align the stack as per the ABI, but pass
-       the original usp to __sigreturn2 as an argument.  */
-    asm volatile ("movq %1, %%rsp\n"
-                  "andq $-16, %%rsp\n"
-                  "call __sigreturn2" :
-                  : "D" (ss), "S" (usp), "d" (scp)
-                  : "memory");
-    __builtin_unreachable ();
-  }
+  /* Copy the registers onto the user's stack, to be able to release the
+     altstack (by unlocking sigstate).  Note that unless an altstack is used,
+     the sigcontext will itself be located on the user's stack, so we may well
+     be overwriting it here (or later in __sigreturn2).
+
+     So: do this very carefully.  First, load sc_reply_port, which is the only
+     other bit of sigcontext that __sigreturn2 needs.  Then copy the registers
+     without reordering them, but skipping the ones we won't need.  We have to
+     copy starting from the larger addresses down, since our register dump is
+     located at a larger address than the sigcontext.  */
+
+  sc_reply_port = scp->sc_reply_port;
+  usp = (uintptr_t *) (scp->sc_ursp - 128);
+
+  *--usp = scp->sc_rip;
+  *--usp = scp->sc_rfl;
+  *--usp = scp->sc_rax;
+  *--usp = scp->sc_rcx;
+  *--usp = scp->sc_rdx;
+  *--usp = scp->sc_rbx;
+  *--usp = scp->sc_rbp;
+  *--usp = scp->sc_rsi;
+  *--usp = scp->sc_rdi;
+  *--usp = scp->sc_r15;
+  *--usp = scp->sc_r14;
+  *--usp = scp->sc_r13;
+  *--usp = scp->sc_r12;
+  *--usp = scp->sc_r11;
+  *--usp = scp->sc_r10;
+  *--usp = scp->sc_r9;
+  *--usp = scp->sc_r8;
+
+  /* Switch to the user's stack that we have just prepared, and call
+     __sigreturn2.  Clobber "memory" to make sure GCC flushes the stack
+     setup to actual memory.  We align the stack as per the ABI, but pass
+     the original usp to __sigreturn2 as an argument.  */
+  asm volatile ("movq %1, %%rsp\n"
+                "andq $-16, %%rsp\n"
+                "call __sigreturn2" :
+                : "D" (ss), "S" (usp), "d" (sc_reply_port)
+                : "memory");
+  __builtin_unreachable ();
 }
 
 weak_alias (__sigreturn, sigreturn)

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-06-04 17:06 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-04 17:06 [glibc] hurd: Fix x86_64 sigreturn restoring bogus reply_port Samuel Thibault

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).