Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 26 Nov 2019 12:06:59 +0100
From:   Oleg Nesterov <oleg@redhat.com>
To:     Andrew Morton <akpm@linux-foundation.org>
Cc:     Andy Lutomirski <luto@kernel.org>, Borislav Petkov <bp@alien8.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>,
        Thomas Gleixner <tglx@linutronix.de>,
        linux-kernel@vger.kernel.org, x86@kernel.org
Subject: [PATCH] ptrace/x86: introduce TS_COMPAT_RESTART to fix
 get_nr_restart_syscall()
Message-ID: <20191126110659.GA14042@redhat.com>
MIME-Version: 1.0
User-Agent: Mutt/1.5.24 (2015-08-30)
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

The comment in get_nr_restart_syscall() says:

=09 * The problem is that we can get here when ptrace pokes
=09 * syscall-like values into regs even if we're not in a syscall
=09 * at all.

Yes. but if we are not in syscall then the

=09status & (TS_COMPAT|TS_I386_REGS_POKED)

check below can't really help:

=09- TS_COMPAT can't be set

=09- TS_I386_REGS_POKED is only set if regs->orig_ax was changed by
=09  32bit debugger; and even in this case get_nr_restart_syscall()
=09  is only correct if the tracee is 32bit too.

Suppose that 64bit debugger plays with 32bit tracee and

=09* Tracee calls sleep(2)=09// TS_COMPAT is set
=09* User interrupts the tracee by CTRL-C after 1 sec and does
=09  "(gdb) call func()"
=09* gdb saves the regs by PTRACE_GETREGS
=09* does PTRACE_SETREGS to set %rip=3D'func' and %orig_rax=3D-1
=09* PTRACE_CONT=09=09// TS_COMPAT is cleared
=09* func() hits int3.
=09* Debugger catches SIGTRAP.
=09* Restore original regs by PTRACE_SETREGS.
=09* PTRACE_CONT

get_nr_restart_syscall() wrongly returns __NR_restart_syscall=3D=3D219, the
tracee calls ia32_sys_call_table[219] =3D=3D sys_madvise.

This patch adds the sticky TS_COMPAT_RESTART flag which survives after
return to user mode, hopefully it allows us to kill TS_I386_REGS_POKED
but this needs another patch.

Test-case:

  $ cvs -d :pserver:anoncvs:anoncvs@sourceware.org:/cvs/systemtap co ptrace=
-tests
  $ gcc -o erestartsys-trap-debuggee ptrace-tests/tests/erestartsys-trap-de=
buggee.c --m32
  $ gcc -o erestartsys-trap-debugger ptrace-tests/tests/erestartsys-trap-de=
bugger.c -lutil
  $ ./erestartsys-trap-debugger
  Unexpected: retval 1, errno 22
  erestartsys-trap-debugger: ptrace-tests/tests/erestartsys-trap-debugger.c=
:421

Reported-by: Jan Kratochvil <jan.kratochvil@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 arch/x86/entry/common.c            | 9 +++++++++
 arch/x86/include/asm/thread_info.h | 1 +
 arch/x86/kernel/signal.c           | 6 ++----
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 3f8e226..1e2c2ca 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -263,6 +263,15 @@ __visible inline void syscall_return_slowpath(struct p=
t_regs *regs)
=20
 =09rseq_syscall(regs);
=20
+#ifdef CONFIG_IA32_EMULATION
+=09if (regs->ax =3D=3D -ERESTART_RESTARTBLOCK) {
+=09=09struct thread_info *ti =3D current_thread_info();
+=09=09if (ti->status & TS_COMPAT)
+=09=09=09ti->status |=3D TS_COMPAT_RESTART;
+=09=09else
+=09=09=09ti->status &=3D ~TS_COMPAT_RESTART;
+=09}
+#endif
 =09/*
 =09 * First do one-time work.  If these work items are enabled, we
 =09 * want to run them exactly once per syscall exit with IRQs on.
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thre=
ad_info.h
index f945353..0784591 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -223,6 +223,7 @@ static inline int arch_within_stack_frames(const void *=
 const stack,
=20
 #ifdef CONFIG_COMPAT
 #define TS_I386_REGS_POKED=090x0004=09/* regs poked by 32-bit ptracer */
+#define TS_COMPAT_RESTART=090x0008
 #endif
 #ifndef __ASSEMBLY__
=20
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 8eb7193..1054f1c 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -788,12 +788,10 @@ static inline unsigned long get_nr_restart_syscall(co=
nst struct pt_regs *regs)
 =09 * For now, we maintain historical behavior and guess based on
 =09 * stored state.  We could do better by saving the actual
 =09 * syscall arch in restart_block or (with caveats on x32) by
-=09 * checking if regs->ip points to 'int $0x80'.  The current
-=09 * behavior is incorrect if a tracer has a different bitness
-=09 * than the tracee.
+=09 * checking if regs->ip points to 'int $0x80'.
 =09 */
 #ifdef CONFIG_IA32_EMULATION
-=09if (current_thread_info()->status & (TS_COMPAT|TS_I386_REGS_POKED))
+=09if (current_thread_info()->status & TS_COMPAT_RESTART)
 =09=09return __NR_ia32_restart_syscall;
 #endif
 #ifdef CONFIG_X86_X32_ABI
--=20
2.5.0