On Tue, Feb 2, 2021 at 7:03 AM Oleg Nesterov <[email protected]> wrote: > > On 02/01, Andy Lutomirski wrote: > > > > On Mon, Feb 1, 2021 at 9:47 AM Oleg Nesterov <[email protected]> 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...
My idea may well be nuts, but I was indeed imagining that we hide ERESTART_BLOCK from the debugger. We would do the ip -= 2 and nr = __NR_..._restart_syscall before any ptrace events happen at all. Admittedly, this may cause strace to fail, so this is probably a bad idea. Oh well, your patch is probably a decent solution.
