From: Samuel Thibault <samuel.thibault@gnu.org>
To: Sergey Bugaev <bugaevc@gmail.com>
Cc: libc-alpha@sourceware.org, bug-hurd@gnu.org
Subject: Re: [PATCH 1/5] hurd: Fix restoring reply port in sigreturn
Date: Mon, 17 Apr 2023 21:00:24 +0200 [thread overview]
Message-ID: <20230417190024.4otllbmss3iaqtf7@begin> (raw)
In-Reply-To: <20230414193700.542116-1-bugaevc@gmail.com>
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
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
prev parent reply other threads:[~2023-04-17 19:00 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-14 19:36 Sergey Bugaev
2023-04-14 19:36 ` [PATCH 2/5] hurd: Microoptimize sigreturn Sergey Bugaev
2023-04-17 23:23 ` Samuel Thibault
2023-04-18 9:34 ` Sergey Bugaev
2023-04-18 10:21 ` [PATCH 2/5] hurd: Microoptimize sigreturn Sergey Bugaev
2023-04-18 14:21 ` Samuel Thibault
2023-04-14 19:36 ` [PATCH 3/5] hurd: Implement sigreturn for x86_64 Sergey Bugaev
2023-04-14 19:36 ` [PATCH 4/5] hurd: Simplify _S_catch_exception_raise Sergey Bugaev
2023-04-17 23:24 ` Samuel Thibault
2023-04-14 19:37 ` [PATCH 5/5] hurd: Avoid leaking task & thread ports Sergey Bugaev
2023-04-17 23:24 ` Samuel Thibault
2023-04-17 19:00 ` Samuel Thibault [this message]
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=20230417190024.4otllbmss3iaqtf7@begin \
--to=samuel.thibault@gnu.org \
--cc=bug-hurd@gnu.org \
--cc=bugaevc@gmail.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).