[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[hurd, commited 3/3] hurd: Fix race between calling RPC and handling a s
|
From: |
Samuel Thibault |
|
Subject: |
[hurd, commited 3/3] hurd: Fix race between calling RPC and handling a signal |
|
Date: |
Sun, 28 Oct 2018 14:38:20 +0100 |
* sysdeps/mach/hurd/i386/intr-msg.h (INTR_MSG_TRAP): Make
_hurd_intr_rpc_msg_about_to global point to start of controlled
assembly snippet. Make it check canceled flag.
* hurd/hurdsig.c (_hurdsig_abort_rpcs): Only mutate thread if it passed
the _hurd_intr_rpc_msg_about_to point.
* hurd/intr-msg.c (_hurd_intr_rpc_mach_msg): Remove comment on mutation
issue, remove cancel flag check.
---
ChangeLog | 7 +++++++
hurd/hurdsig.c | 4 +++-
hurd/intr-msg.c | 17 ++---------------
sysdeps/mach/hurd/i386/intr-msg.h | 23 ++++++++++++++++-------
4 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 7d23aa7236..5709c3cdbc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,13 @@
* hurd/hurdsig.c (_hurd_interrupted_rpc_timeout): Set to 60000.
* hurd/intr-msg.c (_hurd_intr_rpc_mach_msg): When the server does not
answer to interrupt_operation, return EIO instead of EINTR.
+ * sysdeps/mach/hurd/i386/intr-msg.h (INTR_MSG_TRAP): Make
+ _hurd_intr_rpc_msg_about_to global point to start of controlled
+ assembly snippet. Make it check canceled flag.
+ * hurd/hurdsig.c (_hurdsig_abort_rpcs): Only mutate thread if it passed
+ the _hurd_intr_rpc_msg_about_to point.
+ * hurd/intr-msg.c (_hurd_intr_rpc_mach_msg): Remove comment on mutation
+ issue, remove cancel flag check.
2018-10-26 Joseph Myers <address@hidden>
diff --git a/hurd/hurdsig.c b/hurd/hurdsig.c
index 48179b4197..d105615e42 100644
--- a/hurd/hurdsig.c
+++ b/hurd/hurdsig.c
@@ -292,6 +292,7 @@ _hurdsig_abort_rpcs (struct hurd_sigstate *ss, int signo,
int sigthread,
struct machine_thread_all_state *state, int *state_change,
void (*reply) (void))
{
+ extern const void _hurd_intr_rpc_msg_about_to;
extern const void _hurd_intr_rpc_msg_in_trap;
mach_port_t rcv_port = MACH_PORT_NULL;
mach_port_t intr_port;
@@ -307,7 +308,8 @@ _hurdsig_abort_rpcs (struct hurd_sigstate *ss, int signo,
int sigthread,
receive completes immediately or aborts. */
abort_thread (ss, state, reply);
- if (state->basic.PC < (natural_t) &_hurd_intr_rpc_msg_in_trap)
+ if (state->basic.PC >= (natural_t) &_hurd_intr_rpc_msg_about_to &&
+ state->basic.PC < (natural_t) &_hurd_intr_rpc_msg_in_trap)
{
/* The thread is about to do the RPC, but hasn't yet entered
mach_msg. Mutate the thread's state so it knows not to try
diff --git a/hurd/intr-msg.c b/hurd/intr-msg.c
index 1f7724ee8b..7ace0a16b3 100644
--- a/hurd/intr-msg.c
+++ b/hurd/intr-msg.c
@@ -114,23 +114,10 @@ _hurd_intr_rpc_mach_msg (mach_msg_header_t *msg,
message:
- /* XXX
- At all points here (once SS->intr_port is set), the signal thread
- thinks we are "about to enter the syscall", and might mutate our
- return-value register. This is bogus.
- */
-
- if (ss->cancel)
- {
- /* We have been cancelled. Don't do an RPC at all. */
- ss->intr_port = MACH_PORT_NULL;
- ss->cancel = 0;
- return EINTR;
- }
-
/* Note that the signal trampoline code might modify our OPTION! */
err = INTR_MSG_TRAP (msg, option, send_size,
- rcv_size, rcv_name, timeout, notify);
+ rcv_size, rcv_name, timeout, notify,
+ &ss->cancel, &ss->intr_port);
switch (err)
{
diff --git a/sysdeps/mach/hurd/i386/intr-msg.h
b/sysdeps/mach/hurd/i386/intr-msg.h
index 64f05f8c4e..7788c3b892 100644
--- a/sysdeps/mach/hurd/i386/intr-msg.h
+++ b/sysdeps/mach/hurd/i386/intr-msg.h
@@ -20,21 +20,30 @@
/* Note that we must mark OPTION and TIMEOUT as outputs of this operation,
to indicate that the signal thread might mutate them as part
of sending us to a signal handler. */
-#define INTR_MSG_TRAP(msg, option, send_size, rcv_size, rcv_name, timeout,
notify) \
+
+/* After _hurd_intr_rpc_msg_about_to we need to make a last check of cancel, in
+ case we got interrupted right before _hurd_intr_rpc_msg_about_to. */
+#define INTR_MSG_TRAP(msg, option, send_size, rcv_size, rcv_name, timeout,
notify, cancel_p, intr_port_p) \
({ \
error_t err; \
- asm (".globl _hurd_intr_rpc_msg_do_trap\n" \
- ".globl _hurd_intr_rpc_msg_in_trap\n" \
+ asm (".globl _hurd_intr_rpc_msg_about_to\n" \
".globl _hurd_intr_rpc_msg_cx_sp\n" \
+ ".globl _hurd_intr_rpc_msg_do_trap\n" \
+ ".globl _hurd_intr_rpc_msg_in_trap\n" \
".globl _hurd_intr_rpc_msg_sp_restored\n" \
- " movl %%esp, %%ecx\n" \
- " leal %3, %%esp\n" \
+ "_hurd_intr_rpc_msg_about_to: cmpl $0, %5\n" \
+ " jz _hurd_intr_rpc_msg_do\n" \
+ " movl $0, %3\n" \
+ " movl %6, %%eax\n" \
+ " jmp _hurd_intr_rpc_msg_sp_restored\n" \
+ "_hurd_intr_rpc_msg_do: movl %%esp, %%ecx\n" \
+ " leal %4, %%esp\n" \
"_hurd_intr_rpc_msg_cx_sp: movl $-25, %%eax\n" \
"_hurd_intr_rpc_msg_do_trap: lcall $7, $0 # status in %0\n" \
"_hurd_intr_rpc_msg_in_trap: movl %%ecx, %%esp\n" \
"_hurd_intr_rpc_msg_sp_restored:" \
- : "=a" (err), "+m" (option), "+m" (timeout) \
- : "m" ((&msg)[-1]) \
+ : "=a" (err), "+m" (option), "+m" (timeout), "=m" (*intr_port_p)
\
+ : "m" ((&msg)[-1]), "m" (*cancel_p), "i" (EINTR)
\
: "ecx"); \
err; \
})
--
2.19.1