[PATCH v3 0/4] ptrace: cleanup PTRACE_SYSEMU handling and add support for arm64

[PATCH v3 0/4] ptrace: cleanup PTRACE_SYSEMU handling and add support for arm64

[Sudeep Holla]

Hi,

This patchset evolved from the discussion in the thread[0][1]. When we
wanted to add PTRACE_SYSEMU support to ARM64, we thought instead of
duplicating what other architectures like x86 and powerpc have done,
let consolidate the existing support and move it to the core as there's
nothing arch specific in it.

v2->v3:
	- moved clearing of TIF_SYSCALL_EMU to __ptrace_unlink as Oleg
	  suggested
	- x86 cleanup as per Oleg's suggestion and dropped adding new
	  ptrace_syscall_enter for SYSEMU handling
	  (tested using tools/testing/selftests/x86/ptrace_syscall.c)
	- Updated arm64 handling accordingly

v1->v2:
	- added comment for empty statement after tracehook_report_syscall_entry
	- dropped x86 change in syscall_slow_exit_work as I had ended
	  up changing logic unintentionally
	- removed spurious change in powerpc moving user_exit()

Regards,
Sudeep

[0] https://patchwork.kernel.org/patch/10585505/
[1] https://patchwork.kernel.org/patch/10675237/

Sudeep Holla (4):
  ptrace: move clearing of TIF_SYSCALL_EMU flag to core
  x86: simplify _TIF_SYSCALL_EMU handling
  arm64: add PTRACE_SYSEMU{,SINGLESTEP} definations to uapi headers
  arm64: ptrace: add support for syscall emulation

 arch/arm64/include/asm/thread_info.h |  5 ++++-
 arch/arm64/include/uapi/asm/ptrace.h |  3 +++
 arch/arm64/kernel/ptrace.c           |  6 +++++-
 arch/powerpc/kernel/ptrace.c         |  1 -
 arch/x86/entry/common.c              | 17 ++++++-----------
 arch/x86/kernel/ptrace.c             |  3 ---
 kernel/ptrace.c                      |  3 +++
 7 files changed, 21 insertions(+), 17 deletions(-)

--
2.17.1

[PATCH v3 1/4] ptrace: move clearing of TIF_SYSCALL_EMU flag to core

[Sudeep Holla]

While the TIF_SYSCALL_EMU is set in ptrace_resume independent of any
architecture, currently only powerpc and x86 unset the TIF_SYSCALL_EMU
flag in ptrace_disable which gets called from ptrace_detach.

Let's move the clearing of TIF_SYSCALL_EMU flag to __ptrace_unlink
which gets executed from ptrace_detach and also keep it along with
or close to clearing of TIF_SYSCALL_TRACE.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/powerpc/kernel/ptrace.c | 1 -
 arch/x86/kernel/ptrace.c     | 3 ---
 kernel/ptrace.c              | 3 +++
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index d9ac7d94656e..2e2183b800a8 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -2520,7 +2520,6 @@ void ptrace_disable(struct task_struct *child)
 {
 	/* make sure the single step bit is not set. */
 	user_disable_single_step(child);
-	clear_tsk_thread_flag(child, TIF_SYSCALL_EMU);
 }
 
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 4b8ee05dd6ad..45792dbd2443 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -746,9 +746,6 @@ static int ioperm_get(struct task_struct *target,
 void ptrace_disable(struct task_struct *child)
 {
 	user_disable_single_step(child);
-#ifdef TIF_SYSCALL_EMU
-	clear_tsk_thread_flag(child, TIF_SYSCALL_EMU);
-#endif
 }
 
 #if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 6f357f4fc859..16c7fc1eabcf 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -117,6 +117,9 @@ void __ptrace_unlink(struct task_struct *child)
 	BUG_ON(!child->ptrace);
 
 	clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
+#ifdef TIF_SYSCALL_EMU
+	clear_tsk_thread_flag(child, TIF_SYSCALL_EMU);
+#endif
 
 	child->parent = child->real_parent;
 	list_del_init(&child->ptrace_entry);
-- 
2.17.1

[PATCH v3 2/4] x86: simplify _TIF_SYSCALL_EMU handling

[Sudeep Holla]

The usage of emulated/_TIF_SYSCALL_EMU flags in syscall_trace_enter
seems to be bit overcomplicated than required. Let's simplify it.

Cc: Andy Lutomirski <luto@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/x86/entry/common.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 7bc105f47d21..c647ddfd5579 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -70,23 +70,18 @@ static long syscall_trace_enter(struct pt_regs *regs)
 
 	struct thread_info *ti = current_thread_info();
 	unsigned long ret = 0;
-	bool emulated = false;
 	u32 work;
 
 	if (IS_ENABLED(CONFIG_DEBUG_ENTRY))
 		BUG_ON(regs != task_pt_regs(current));
 
-	work = READ_ONCE(ti->flags) & _TIF_WORK_SYSCALL_ENTRY;
+	work = READ_ONCE(ti->flags);
 
-	if (unlikely(work & _TIF_SYSCALL_EMU))
-		emulated = true;
-
-	if ((emulated || (work & _TIF_SYSCALL_TRACE)) &&
-	    tracehook_report_syscall_entry(regs))
-		return -1L;
-
-	if (emulated)
-		return -1L;
+	if (work & (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_EMU)) {
+		ret = tracehook_report_syscall_entry(regs);
+		if (ret || (work & _TIF_SYSCALL_EMU))
+			return -1L;
+	}
 
 #ifdef CONFIG_SECCOMP
 	/*
-- 
2.17.1

[PATCH v3 3/4] arm64: add PTRACE_SYSEMU{,SINGLESTEP} definations to uapi headers

[Sudeep Holla]

x86 and um use 31 and 32 for PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP
while powerpc uses different value maybe for legacy reasons.

Though handling of PTRACE_SYSEMU can be made architecture independent,
it's hard to make these definations generic. To add to this existing
mess few architectures like arm, c6x and sh use 31 for PTRACE_GETFDPIC
(get the ELF fdpic loadmap address). It's not possible to move the
definations to generic headers.

So we unfortunately have to duplicate the same defination to ARM64 if
we need to support PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/include/uapi/asm/ptrace.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index d78623acb649..627ac57c1581 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -62,6 +62,9 @@
 #define PSR_x		0x0000ff00	/* Extension		*/
 #define PSR_c		0x000000ff	/* Control		*/
 
+/* syscall emulation path in ptrace */
+#define PTRACE_SYSEMU		  31
+#define PTRACE_SYSEMU_SINGLESTEP  32
 
 #ifndef __ASSEMBLY__
 
-- 
2.17.1

[PATCH v3 4/4] arm64: ptrace: add support for syscall emulation

[Sudeep Holla]

Add PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP support on arm64.
We don't need any special handling for PTRACE_SYSEMU_SINGLESTEP.

It's quite difficult to generalize handling PTRACE_SYSEMU cross
architectures and avoid calls to tracehook_report_syscall_entry twice.
Different architecture have different mechanism to indicate NO_SYSCALL
and trying to generalise adds more code for no gain.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/include/asm/thread_info.h | 5 ++++-
 arch/arm64/kernel/ptrace.c           | 6 +++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

Hi Will,

While I agree with your idea of consolidation of PTRACE_SYSEMU handling
to core, it's quite a lot of change to do that, though it may be simple
and confusing with indirection from arch code to core.

Regards,
Sudeep

diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index eb3ef73e07cf..c285d1ce7186 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -75,6 +75,7 @@ void arch_release_task_struct(struct task_struct *tsk);
  *  TIF_SYSCALL_TRACE	- syscall trace active
  *  TIF_SYSCALL_TRACEPOINT - syscall tracepoint for ftrace
  *  TIF_SYSCALL_AUDIT	- syscall auditing
+ *  TIF_SYSCALL_EMU     - syscall emulation active
  *  TIF_SECOMP		- syscall secure computing
  *  TIF_SIGPENDING	- signal pending
  *  TIF_NEED_RESCHED	- rescheduling necessary
@@ -91,6 +92,7 @@ void arch_release_task_struct(struct task_struct *tsk);
 #define TIF_SYSCALL_AUDIT	9
 #define TIF_SYSCALL_TRACEPOINT	10
 #define TIF_SECCOMP		11
+#define TIF_SYSCALL_EMU		12
 #define TIF_MEMDIE		18	/* is terminating due to OOM killer */
 #define TIF_FREEZE		19
 #define TIF_RESTORE_SIGMASK	20
@@ -109,6 +111,7 @@ void arch_release_task_struct(struct task_struct *tsk);
 #define _TIF_SYSCALL_AUDIT	(1 << TIF_SYSCALL_AUDIT)
 #define _TIF_SYSCALL_TRACEPOINT	(1 << TIF_SYSCALL_TRACEPOINT)
 #define _TIF_SECCOMP		(1 << TIF_SECCOMP)
+#define _TIF_SYSCALL_EMU	(1 << TIF_SYSCALL_EMU)
 #define _TIF_UPROBE		(1 << TIF_UPROBE)
 #define _TIF_FSCHECK		(1 << TIF_FSCHECK)
 #define _TIF_32BIT		(1 << TIF_32BIT)
@@ -120,7 +123,7 @@ void arch_release_task_struct(struct task_struct *tsk);

 #define _TIF_SYSCALL_WORK	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
 				 _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP | \
-				 _TIF_NOHZ)
+				 _TIF_NOHZ | _TIF_SYSCALL_EMU)

 #define INIT_THREAD_INFO(tsk)						\
 {									\
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index b82e0a9b3da3..9353355cb91a 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -1819,8 +1819,12 @@ static void tracehook_report_syscall(struct pt_regs *regs,

 int syscall_trace_enter(struct pt_regs *regs)
 {
-	if (test_thread_flag(TIF_SYSCALL_TRACE))
+	if (test_thread_flag(TIF_SYSCALL_TRACE) ||
+		test_thread_flag(TIF_SYSCALL_EMU)) {
 		tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
+		if (!in_syscall(regs) || test_thread_flag(TIF_SYSCALL_EMU))
+			return -1;
+	}

 	/* Do the secure computing after ptrace; failures should be fast. */
 	if (secure_computing(NULL) == -1)
--
2.17.1

Re: [PATCH v3 2/4] x86: simplify _TIF_SYSCALL_EMU handling

[Oleg Nesterov]

On 04/30, Sudeep Holla wrote:
>
> The usage of emulated/_TIF_SYSCALL_EMU flags in syscall_trace_enter
> seems to be bit overcomplicated than required. Let's simplify it.
>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Re: [PATCH v3 1/4] ptrace: move clearing of TIF_SYSCALL_EMU flag to core

[Oleg Nesterov]

On 04/30, Sudeep Holla wrote:
>
> While the TIF_SYSCALL_EMU is set in ptrace_resume independent of any
> architecture, currently only powerpc and x86 unset the TIF_SYSCALL_EMU
> flag in ptrace_disable which gets called from ptrace_detach.
>
> Let's move the clearing of TIF_SYSCALL_EMU flag to __ptrace_unlink
> which gets executed from ptrace_detach and also keep it along with
> or close to clearing of TIF_SYSCALL_TRACE.
>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Oleg Nesterov <oleg@redhat.com>

Re: [PATCH v3 1/4] ptrace: move clearing of TIF_SYSCALL_EMU flag to core

[Sudeep Holla]

On Wed, May 01, 2019 at 06:13:30PM +0200, Oleg Nesterov wrote:
> On 04/30, Sudeep Holla wrote:
> >
> > While the TIF_SYSCALL_EMU is set in ptrace_resume independent of any
> > architecture, currently only powerpc and x86 unset the TIF_SYSCALL_EMU
> > flag in ptrace_disable which gets called from ptrace_detach.
> >
> > Let's move the clearing of TIF_SYSCALL_EMU flag to __ptrace_unlink
> > which gets executed from ptrace_detach and also keep it along with
> > or close to clearing of TIF_SYSCALL_TRACE.
> >
> > Cc: Oleg Nesterov <oleg@redhat.com>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> 
> Acked-by: Oleg Nesterov <oleg@redhat.com>
>

Since 1/4 and 2/4 are completely independent of arm64 changes in 3&4/4,
I prefer you take these via your tree.

--
Regards,
Sudeep

Re: [PATCH v3 1/4] ptrace: move clearing of TIF_SYSCALL_EMU flag to core

[Oleg Nesterov]

On 05/01, Sudeep Holla wrote:
>
> On Wed, May 01, 2019 at 06:13:30PM +0200, Oleg Nesterov wrote:
> > On 04/30, Sudeep Holla wrote:
> > >
> > > While the TIF_SYSCALL_EMU is set in ptrace_resume independent of any
> > > architecture, currently only powerpc and x86 unset the TIF_SYSCALL_EMU
> > > flag in ptrace_disable which gets called from ptrace_detach.
> > >
> > > Let's move the clearing of TIF_SYSCALL_EMU flag to __ptrace_unlink
> > > which gets executed from ptrace_detach and also keep it along with
> > > or close to clearing of TIF_SYSCALL_TRACE.
> > >
> > > Cc: Oleg Nesterov <oleg@redhat.com>
> > > Cc: Paul Mackerras <paulus@samba.org>
> > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Ingo Molnar <mingo@redhat.com>
> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> >
> > Acked-by: Oleg Nesterov <oleg@redhat.com>
> >
>
> Since 1/4 and 2/4 are completely independent of arm64 changes in 3&4/4,
> I prefer you take these via your tree.

Sorry Sudeep, I can't do this, I need to reanimate my account on kernel.org.

Oleg.

Re: [PATCH v3 1/4] ptrace: move clearing of TIF_SYSCALL_EMU flag to core

[Will Deacon]

On Thu, May 02, 2019 at 06:13:30PM +0200, Oleg Nesterov wrote:
> On 05/01, Sudeep Holla wrote:
> >
> > On Wed, May 01, 2019 at 06:13:30PM +0200, Oleg Nesterov wrote:
> > > On 04/30, Sudeep Holla wrote:
> > > >
> > > > While the TIF_SYSCALL_EMU is set in ptrace_resume independent of any
> > > > architecture, currently only powerpc and x86 unset the TIF_SYSCALL_EMU
> > > > flag in ptrace_disable which gets called from ptrace_detach.
> > > >
> > > > Let's move the clearing of TIF_SYSCALL_EMU flag to __ptrace_unlink
> > > > which gets executed from ptrace_detach and also keep it along with
> > > > or close to clearing of TIF_SYSCALL_TRACE.
> > > >
> > > > Cc: Oleg Nesterov <oleg@redhat.com>
> > > > Cc: Paul Mackerras <paulus@samba.org>
> > > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > > Cc: Ingo Molnar <mingo@redhat.com>
> > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > >
> > > Acked-by: Oleg Nesterov <oleg@redhat.com>
> > >
> >
> > Since 1/4 and 2/4 are completely independent of arm64 changes in 3&4/4,
> > I prefer you take these via your tree.
> 
> Sorry Sudeep, I can't do this, I need to reanimate my account on kernel.org.

Ok, if you're happy for us to take them via arm64 with your ack, then we can
do that as well. Just don't want to step on anybody's toes!

Will

Re: [PATCH v3 1/4] ptrace: move clearing of TIF_SYSCALL_EMU flag to core

[Oleg Nesterov]

On 05/02, Will Deacon wrote:
>
> Ok, if you're happy for us to take them via arm64 with your ack, then we can
> do that as well.

Yes, yes, please!

Oleg.