intr-msg / hurdsig looks broken, is my analysis correct?

[Sergey Bugaev]

On Mon, Feb 27, 2023 at 7:39 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > Are there any tests for this at all?
>
> It's very difficult to test since it's all about race conditions, with
> the interruption happening exactly at the wrong moment.

Right, but can't it be made reliable with some creative use of
ptrace/thread_state APIs? Set a breakpoint like gdb would do (by
inserting an int3...) on an instruction, have the thread run up to
there and stop, then send it a singal. Repeat for each instruction, to
make sure everything works no matter what state we catch the thread
in. Granted, this would not be trivial, but sounds doable & useful,
no? Not that I volunteer to implement this, of course no :D

> > As for fixing this:
> > - we should drop the ecx kludge indeed, meaning, remove everything
> > related to ecx from all of these files
>
> Actually, we probably need a kludge, precisely because of the pushes.
>
> > - there should be an explicit way for hurdsig.c to tell INTR_MSG_TRAP
> > to not do the syscall.
>
> Yes, but as much as you'll try, you'll still have a very tiny window
> (1-instruction at least) between the moment you check, and the moment
> you run lcall. That's why the whole thing. It's simpler to just reset to
> a known state.

Yes, I was not suggesting to only rely on an explicit flag, of course
that's racy. What I meant is more like:

_hurd_intr_rpc_msg_about_to:
    push stuff...
    any other potential setup...
/* From here on, it's safe to make us jump to after the syscall */
_hurd_intr_rpc_msg_setup_done:
/* Check for explicitly having been told not to do the syscall */
    test %eax, %eax
    jnz _hurd_intr_rpc_msg_in_trap
    potentially do some other stuff in here...
    movl $-25, %eax
_hurd_intr_rpc_msg_do_trap:
    lcall $7, $0
_hurd_intr_rpc_msg_in_trap:
    pop stuff...

If we are stopped *before* _hurd_intr_rpc_msg_setup_done, hurdsig.c
would know we are still to test %eax, %eax, so it can set eax = 1 and
resume us. If we are stopped *after* _hurd_intr_rpc_msg_setup_done, it
is safe to jump over the syscall, so it sets eip =
_hurd_intr_rpc_msg_in_trap.

> We can use ecx for that, by saving esp to ecx before the pushes.
> If interruption happens between the start of the pushes and the lcall,
> we can easily revert the pushes by copying ecx to esp (i.e. without
> having to care exactly how many pushes did get achieved), and directly
> branch to _hurd_intr_rpc_msg_sp_restored.

But you're right, that sounds like it would work too.

I shall take a shot at implementing it then :)

Sergey