Message ID | 20230414193700.542116-1-bugaevc@gmail.com |
---|---|
State | New |
Headers | show |
Series | [1/5] hurd: Fix restoring reply port in sigreturn | expand |
Applied, thanks! Sergey Bugaev, le ven. 14 avril 2023 22:36:56 +0300, a ecrit: > We must not use the user's reply port (scp->sc_reply_port) for any of > our own RPCs, otherwise various things break. So, use MACH_PORT_DEAD as > a reply port when destroying our reply port, and make sure to do this > after _hurd_sigstate_unlock (), which may do a gsync_wake () RPC. > > Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> > --- > sysdeps/mach/hurd/i386/sigreturn.c | 35 ++++++++++++++++++++---------- > 1 file changed, 23 insertions(+), 12 deletions(-) > > diff --git a/sysdeps/mach/hurd/i386/sigreturn.c b/sysdeps/mach/hurd/i386/sigreturn.c > index 4f196710..a0fc8891 100644 > --- a/sysdeps/mach/hurd/i386/sigreturn.c > +++ b/sysdeps/mach/hurd/i386/sigreturn.c > @@ -26,11 +26,29 @@ register int *sp asm ("%esp"); > /* This is run on the thread stack after restoring it, to be able to > unlock SS off sigstack. */ > static void > -__sigreturn2 (int *usp) > +__sigreturn2 (int *usp, struct sigcontext *scp) > { > + mach_port_t reply_port; > struct hurd_sigstate *ss = _hurd_self_sigstate (); > _hurd_sigstate_unlock (ss); > > + /* Destroy the MiG reply port used by the signal handler, and restore the > + reply port in use by the thread when interrupted. > + > + We cannot use the original reply port for our RPCs that we do here, since > + we could unexpectedly receive/consume a reply message meant for the user > + (in particular, msg_sig_post_reply), and also since we would deallocate > + the port if *our* RPC fails, which we don't want to do since the user > + still has the old name. And so, temporarily set MACH_PORT_DEAD as our > + reply name, and make sure destroying the port is the very last RPC we > + do. */ > + reply_port = THREAD_GETMEM (THREAD_SELF, reply_port); > + THREAD_SETMEM (THREAD_SELF, reply_port, MACH_PORT_DEAD); > + if (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); > + > sp = usp; > #define A(line) asm volatile (#line) > /* The members in the sigcontext are arranged in this order > @@ -58,7 +76,6 @@ __sigreturn (struct sigcontext *scp) > { > struct hurd_sigstate *ss; > struct hurd_userlink *link = (void *) &scp[1]; > - mach_port_t reply_port; > > if (scp == NULL || (scp->sc_mask & _SIG_CANT_MASK)) > { > @@ -98,13 +115,6 @@ __sigreturn (struct sigcontext *scp) > if (scp->sc_onstack) > ss->sigaltstack.ss_flags &= ~SS_ONSTACK; > > - /* Destroy the MiG reply port used by the signal handler, and restore the > - reply port in use by the thread when interrupted. */ > - reply_port = THREAD_GETMEM (THREAD_SELF, reply_port); > - THREAD_SETMEM (THREAD_SELF, reply_port, scp->sc_reply_port); > - if (MACH_PORT_VALID (reply_port)) > - __mach_port_destroy (__mach_task_self (), reply_port); > - > if (scp->sc_fpused) > /* Restore the FPU state. Mach conveniently stores the state > in the format the i387 `frstor' instruction uses to restore it. */ > @@ -115,15 +125,16 @@ __sigreturn (struct sigcontext *scp) > copy the registers onto the user's stack, switch there, pop and > return. */ > > - int *usp = (int *) scp->sc_uesp; > + int usp_arg, *usp = (int *) scp->sc_uesp; > > *--usp = scp->sc_eip; > *--usp = scp->sc_efl; > memcpy (usp -= 12, &scp->sc_i386_thread_state, 12 * sizeof (int)); > + usp_arg = (int) usp; > > + *--usp = (int) scp; > /* Pass usp to __sigreturn2 so it can unwind itself easily. */ > - *(usp-1) = (int) usp; > - --usp; > + *--usp = usp_arg; > /* Bogus return address for __sigreturn2 */ > *--usp = 0; > *--usp = (int) __sigreturn2; > -- > 2.39.2 >
diff --git a/sysdeps/mach/hurd/i386/sigreturn.c b/sysdeps/mach/hurd/i386/sigreturn.c index 4f196710..a0fc8891 100644 --- a/sysdeps/mach/hurd/i386/sigreturn.c +++ b/sysdeps/mach/hurd/i386/sigreturn.c @@ -26,11 +26,29 @@ register int *sp asm ("%esp"); /* This is run on the thread stack after restoring it, to be able to unlock SS off sigstack. */ static void -__sigreturn2 (int *usp) +__sigreturn2 (int *usp, struct sigcontext *scp) { + mach_port_t reply_port; struct hurd_sigstate *ss = _hurd_self_sigstate (); _hurd_sigstate_unlock (ss); + /* Destroy the MiG reply port used by the signal handler, and restore the + reply port in use by the thread when interrupted. + + We cannot use the original reply port for our RPCs that we do here, since + we could unexpectedly receive/consume a reply message meant for the user + (in particular, msg_sig_post_reply), and also since we would deallocate + the port if *our* RPC fails, which we don't want to do since the user + still has the old name. And so, temporarily set MACH_PORT_DEAD as our + reply name, and make sure destroying the port is the very last RPC we + do. */ + reply_port = THREAD_GETMEM (THREAD_SELF, reply_port); + THREAD_SETMEM (THREAD_SELF, reply_port, MACH_PORT_DEAD); + if (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); + sp = usp; #define A(line) asm volatile (#line) /* The members in the sigcontext are arranged in this order @@ -58,7 +76,6 @@ __sigreturn (struct sigcontext *scp) { struct hurd_sigstate *ss; struct hurd_userlink *link = (void *) &scp[1]; - mach_port_t reply_port; if (scp == NULL || (scp->sc_mask & _SIG_CANT_MASK)) { @@ -98,13 +115,6 @@ __sigreturn (struct sigcontext *scp) if (scp->sc_onstack) ss->sigaltstack.ss_flags &= ~SS_ONSTACK; - /* Destroy the MiG reply port used by the signal handler, and restore the - reply port in use by the thread when interrupted. */ - reply_port = THREAD_GETMEM (THREAD_SELF, reply_port); - THREAD_SETMEM (THREAD_SELF, reply_port, scp->sc_reply_port); - if (MACH_PORT_VALID (reply_port)) - __mach_port_destroy (__mach_task_self (), reply_port); - if (scp->sc_fpused) /* Restore the FPU state. Mach conveniently stores the state in the format the i387 `frstor' instruction uses to restore it. */ @@ -115,15 +125,16 @@ __sigreturn (struct sigcontext *scp) copy the registers onto the user's stack, switch there, pop and return. */ - int *usp = (int *) scp->sc_uesp; + int usp_arg, *usp = (int *) scp->sc_uesp; *--usp = scp->sc_eip; *--usp = scp->sc_efl; memcpy (usp -= 12, &scp->sc_i386_thread_state, 12 * sizeof (int)); + usp_arg = (int) usp; + *--usp = (int) scp; /* Pass usp to __sigreturn2 so it can unwind itself easily. */ - *(usp-1) = (int) usp; - --usp; + *--usp = usp_arg; /* Bogus return address for __sigreturn2 */ *--usp = 0; *--usp = (int) __sigreturn2;
We must not use the user's reply port (scp->sc_reply_port) for any of our own RPCs, otherwise various things break. So, use MACH_PORT_DEAD as a reply port when destroying our reply port, and make sure to do this after _hurd_sigstate_unlock (), which may do a gsync_wake () RPC. Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> --- sysdeps/mach/hurd/i386/sigreturn.c | 35 ++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 12 deletions(-)