All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andy Lutomirski <luto@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>,
	Jan Kratochvil <jan.kratochvil@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Pedro Alves <palves@redhat.com>, Peter Anvin <hpa@zytor.com>,
	LKML <linux-kernel@vger.kernel.org>, X86 ML <x86@kernel.org>
Subject: Re: [PATCH v4 3/4] x86: introduce TS_COMPAT_RESTART to fix
Date: Tue, 2 Feb 2021 16:02:48 +0100	[thread overview]
Message-ID: <20210202150247.GA20059@redhat.com> (raw)
In-Reply-To: <CALCETrWrPyd1HLXfKLc17CF85r2336YoEpe6bo6dNGdG_2A2bQ@mail.gmail.com>

On 02/01, Andy Lutomirski wrote:
>
> On Mon, Feb 1, 2021 at 9:47 AM Oleg Nesterov <oleg@redhat.com> 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.


  reply	other threads:[~2021-02-02 15:06 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-01 17:45 [PATCH RESEND v4 0/4] x86: fix get_nr_restart_syscall() Oleg Nesterov
2021-02-01 17:46 ` [PATCH v4 1/4] introduce set_restart_fn() and arch_set_restart_data() Oleg Nesterov
2021-03-16 21:17   ` [tip: x86/urgent] kernel, fs: Introduce and use " tip-bot2 for Oleg Nesterov
2021-02-01 17:46 ` [PATCH v4 2/4] x86: mv TS_COMPAT from asm/processor.h to Oleg Nesterov
2021-03-16 21:17   ` [tip: x86/urgent] x86: Move TS_COMPAT back to asm/thread_info.h tip-bot2 for Oleg Nesterov
2021-02-01 17:47 ` [PATCH v4 3/4] x86: introduce TS_COMPAT_RESTART to fix Oleg Nesterov
2021-02-01 18:32   ` Andy Lutomirski
2021-02-02 15:02     ` Oleg Nesterov [this message]
2021-02-02 17:27       ` Andy Lutomirski
2021-03-16 21:17   ` [tip: x86/urgent] x86: Introduce TS_COMPAT_RESTART to fix get_nr_restart_syscall() tip-bot2 for Oleg Nesterov
2021-02-01 17:47 ` [PATCH v4 4/4] x86: introduce restart_block->arch_data to kill Oleg Nesterov
2021-03-16 21:17   ` [tip: x86/urgent] x86: Introduce restart_block->arch_data to remove TS_COMPAT_RESTART tip-bot2 for Oleg Nesterov
2021-02-01 18:18 ` [PATCH RESEND v4 0/4] x86: fix get_nr_restart_syscall() Linus Torvalds
2021-02-01 18:19   ` Linus Torvalds
2021-02-02 15:55     ` Oleg Nesterov
2021-02-02 18:15       ` Linus Torvalds
2021-02-02 19:23         ` Oleg Nesterov
2021-02-03 23:19 ` Oleg Nesterov
2021-03-16 18:10   ` Oleg Nesterov
2021-03-16 18:26     ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210202150247.GA20059@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=jan.kratochvil@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=palves@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.