hurd: Fix race between calling RPC and handling a signal

* 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.
This commit is contained in:
Samuel Thibault 2018-10-09 23:40:09 +02:00
parent 2d0d1d3876
commit 5c81be5340
4 changed files with 28 additions and 23 deletions

View File

@ -3,6 +3,13 @@
* hurd/hurdsig.c (_hurd_interrupted_rpc_timeout): Set to 60000. * hurd/hurdsig.c (_hurd_interrupted_rpc_timeout): Set to 60000.
* hurd/intr-msg.c (_hurd_intr_rpc_mach_msg): When the server does not * hurd/intr-msg.c (_hurd_intr_rpc_mach_msg): When the server does not
answer to interrupt_operation, return EIO instead of EINTR. 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 <joseph@codesourcery.com> 2018-10-26 Joseph Myers <joseph@codesourcery.com>

View File

@ -292,6 +292,7 @@ _hurdsig_abort_rpcs (struct hurd_sigstate *ss, int signo, int sigthread,
struct machine_thread_all_state *state, int *state_change, struct machine_thread_all_state *state, int *state_change,
void (*reply) (void)) void (*reply) (void))
{ {
extern const void _hurd_intr_rpc_msg_about_to;
extern const void _hurd_intr_rpc_msg_in_trap; extern const void _hurd_intr_rpc_msg_in_trap;
mach_port_t rcv_port = MACH_PORT_NULL; mach_port_t rcv_port = MACH_PORT_NULL;
mach_port_t intr_port; 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. */ receive completes immediately or aborts. */
abort_thread (ss, state, reply); 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 /* 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 mach_msg. Mutate the thread's state so it knows not to try

View File

@ -114,23 +114,10 @@ _hurd_intr_rpc_mach_msg (mach_msg_header_t *msg,
message: 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! */ /* Note that the signal trampoline code might modify our OPTION! */
err = INTR_MSG_TRAP (msg, option, send_size, 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) switch (err)
{ {

View File

@ -20,21 +20,30 @@
/* Note that we must mark OPTION and TIMEOUT as outputs of this operation, /* Note that we must mark OPTION and TIMEOUT as outputs of this operation,
to indicate that the signal thread might mutate them as part to indicate that the signal thread might mutate them as part
of sending us to a signal handler. */ 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; \ error_t err; \
asm (".globl _hurd_intr_rpc_msg_do_trap\n" \ asm (".globl _hurd_intr_rpc_msg_about_to\n" \
".globl _hurd_intr_rpc_msg_in_trap\n" \
".globl _hurd_intr_rpc_msg_cx_sp\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" \ ".globl _hurd_intr_rpc_msg_sp_restored\n" \
" movl %%esp, %%ecx\n" \ "_hurd_intr_rpc_msg_about_to: cmpl $0, %5\n" \
" leal %3, %%esp\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_cx_sp: movl $-25, %%eax\n" \
"_hurd_intr_rpc_msg_do_trap: lcall $7, $0 # status in %0\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_in_trap: movl %%ecx, %%esp\n" \
"_hurd_intr_rpc_msg_sp_restored:" \ "_hurd_intr_rpc_msg_sp_restored:" \
: "=a" (err), "+m" (option), "+m" (timeout) \ : "=a" (err), "+m" (option), "+m" (timeout), "=m" (*intr_port_p) \
: "m" ((&msg)[-1]) \ : "m" ((&msg)[-1]), "m" (*cancel_p), "i" (EINTR) \
: "ecx"); \ : "ecx"); \
err; \ err; \
}) })