Re: [PATCH v4 3/4] x86: introduce TS_COMPAT_RESTART to fix

[Oleg Nesterov]

On 02/01, Andy Lutomirski wrote:
>
> On Mon, Feb 1, 2021 at 9:47 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> >
> > The comment in get_nr_restart_syscall() says:
> >
> >          * The problem is that we can get here when ptrace pokes
> >          * syscall-like values into regs even if we're not in a syscall
> >          * at all.
> >
> > Yes. but if we are not in syscall then the
> >
> >         status & (TS_COMPAT|TS_I386_REGS_POKED)
> >
> > check below can't really help:
> >
> >         - TS_COMPAT can't be set
> >
> >         - TS_I386_REGS_POKED is only set if regs->orig_ax was changed by
> >           32bit debugger; and even in this case get_nr_restart_syscall()
> >           is only correct if the tracee is 32bit too.
> >
> > Suppose that 64bit debugger plays with 32bit tracee and
>
> At the risk of asking an obnoxious question here:
>
> >
> >         * Tracee calls sleep(2) // TS_COMPAT is set
> >         * User interrupts the tracee by CTRL-C after 1 sec and does
> >           "(gdb) call func()"
> >         * gdb saves the regs by PTRACE_GETREGS
>
> It seems to me that a better solution may be for gdb to see the
> post-restart-setup state.  In other words, shouldn't the GETREGS
> return with the ax pointing to the restart syscall already?

and ip = regs-ip - 2? And hide ERESTART_BLOCK from debugger? Perhaps
I misunderstood, but this doesn't look like a better solution to me.
Not to mention this would be the serious user-visible change... And
even the necessary changes in getreg() do not look good to me.

Plus I do not understand how this could work. OK, suppose that the
tracee reports a signal with ax = ERESTART_BLOCK.

Debugger simply does GETREGS + SETREGS + PTRACE_CONT(signr). In this
case handle_signal() should set ax = -EINTR, but syscall_get_error()
will report __NR_ia32_restart_syscall?

Probably I greatly misunderstood you...

Oleg.