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

[Samuel Thibault]

Sergey Bugaev, le lun. 27 févr. 2023 20:03:07 +0300, a ecrit:
> 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

This looks quite complex and fragile, compared to just mangling the CPU
state...

(not that we haven't ever screwed with the CPU state, but more complex
approaches will be yet more fragile).

> > > 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:

(add movl $0, %eax here)

> _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.

That looks alright to me, indeed, and less kludgy :)

> I shall take a shot at implementing it then :)

Thanks!

Samuel