diff mbox series

[v2,2/6] ptrace: introduce ptrace_syscall_enter to consolidate PTRACE_SYSEMU handling

Message ID 20190318104925.16600-3-sudeep.holla@arm.com
State New
Headers show
Series ptrace: consolidate PTRACE_SYSEMU handling and add support for arm64 | expand

Commit Message

Sudeep Holla March 18, 2019, 10:49 a.m. UTC
Currently each architecture handles PTRACE_SYSEMU in very similar way.
It's completely arch independent and can be handled in the code helping
to consolidate PTRACE_SYSEMU handling.

Let's introduce a hook 'ptrace_syscall_enter' that arch specific syscall
entry code can call.

Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

---
 include/linux/ptrace.h |  1 +
 kernel/ptrace.c        | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+)

-- 
2.17.1

Comments

Dmitry V. Levin March 18, 2019, 2:31 p.m. UTC | #1
On Mon, Mar 18, 2019 at 10:49:21AM +0000, Sudeep Holla wrote:
> Currently each architecture handles PTRACE_SYSEMU in very similar way.

> It's completely arch independent and can be handled in the code helping

> to consolidate PTRACE_SYSEMU handling.

> 

> Let's introduce a hook 'ptrace_syscall_enter' that arch specific syscall

> entry code can call.

> 

> Cc: Oleg Nesterov <oleg@redhat.com>

> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

> ---

>  include/linux/ptrace.h |  1 +

>  kernel/ptrace.c        | 22 ++++++++++++++++++++++

>  2 files changed, 23 insertions(+)

> 

> diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h

> index edb9b040c94c..e30f51e3363e 100644

> --- a/include/linux/ptrace.h

> +++ b/include/linux/ptrace.h

> @@ -407,6 +407,7 @@ static inline void user_single_step_report(struct pt_regs *regs)

>  #define current_user_stack_pointer() user_stack_pointer(current_pt_regs())

>  #endif

>  

> +extern long ptrace_syscall_enter(struct pt_regs *regs);

>  extern int task_current_syscall(struct task_struct *target, long *callno,

>  				unsigned long args[6], unsigned int maxargs,

>  				unsigned long *sp, unsigned long *pc);

> diff --git a/kernel/ptrace.c b/kernel/ptrace.c

> index 4fa3b7f4c3c7..c9c505c483df 100644

> --- a/kernel/ptrace.c

> +++ b/kernel/ptrace.c

> @@ -29,6 +29,7 @@

>  #include <linux/hw_breakpoint.h>

>  #include <linux/cn_proc.h>

>  #include <linux/compat.h>

> +#include <linux/tracehook.h>

>  

>  /*

>   * Access another process' address space via ptrace.

> @@ -557,6 +558,27 @@ static int ptrace_detach(struct task_struct *child, unsigned int data)

>  	return 0;

>  }

>  

> +/*

> + * Hook to check and report for PTRACE_SYSEMU, can be called from arch

> + * arch syscall entry code

> + */

> +long ptrace_syscall_enter(struct pt_regs *regs)

> +{

> +#ifdef TIF_SYSCALL_EMU

> +	if (test_thread_flag(TIF_SYSCALL_EMU)) {

> +		if (tracehook_report_syscall_entry(regs))

> +			/*

> +			 * We can ignore the return code here as we need

> +			 * return -1 always for syscall emulation irrespective

> +			 * of whether the tracehook report fails or succeed.

> +			 */

> +			;


This is problematic as it causes build errors with -Werror=empty-body,
see https://lore.kernel.org/lkml/20181218205305.26647-1-malat@debian.org/


-- 
ldv
Dmitry V. Levin March 18, 2019, 2:41 p.m. UTC | #2
On Mon, Mar 18, 2019 at 10:49:21AM +0000, Sudeep Holla wrote:
> Currently each architecture handles PTRACE_SYSEMU in very similar way.

> It's completely arch independent and can be handled in the code helping

> to consolidate PTRACE_SYSEMU handling.

> 

> Let's introduce a hook 'ptrace_syscall_enter' that arch specific syscall

> entry code can call.


Sorry if I'm late for the party, but the new name looks confusing.
If all it does is related to TIF_SYSCALL_EMU, why does it have a generic
name 'ptrace_syscall_enter' without any hint of being specific to
TIF_SYSCALL_EMU?


-- 
ldv
Sudeep Holla March 18, 2019, 2:55 p.m. UTC | #3
On Mon, Mar 18, 2019 at 05:31:47PM +0300, Dmitry V. Levin wrote:
> On Mon, Mar 18, 2019 at 10:49:21AM +0000, Sudeep Holla wrote:

> > Currently each architecture handles PTRACE_SYSEMU in very similar way.

> > It's completely arch independent and can be handled in the code helping

> > to consolidate PTRACE_SYSEMU handling.

> > 

> > Let's introduce a hook 'ptrace_syscall_enter' that arch specific syscall

> > entry code can call.

> > 

> > Cc: Oleg Nesterov <oleg@redhat.com>

> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

> > ---

> >  include/linux/ptrace.h |  1 +

> >  kernel/ptrace.c        | 22 ++++++++++++++++++++++

> >  2 files changed, 23 insertions(+)

> > 

> > diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h

> > index edb9b040c94c..e30f51e3363e 100644

> > --- a/include/linux/ptrace.h

> > +++ b/include/linux/ptrace.h

> > @@ -407,6 +407,7 @@ static inline void user_single_step_report(struct pt_regs *regs)

> >  #define current_user_stack_pointer() user_stack_pointer(current_pt_regs())

> >  #endif

> >  

> > +extern long ptrace_syscall_enter(struct pt_regs *regs);

> >  extern int task_current_syscall(struct task_struct *target, long *callno,

> >  				unsigned long args[6], unsigned int maxargs,

> >  				unsigned long *sp, unsigned long *pc);

> > diff --git a/kernel/ptrace.c b/kernel/ptrace.c

> > index 4fa3b7f4c3c7..c9c505c483df 100644

> > --- a/kernel/ptrace.c

> > +++ b/kernel/ptrace.c

> > @@ -29,6 +29,7 @@

> >  #include <linux/hw_breakpoint.h>

> >  #include <linux/cn_proc.h>

> >  #include <linux/compat.h>

> > +#include <linux/tracehook.h>

> >  

> >  /*

> >   * Access another process' address space via ptrace.

> > @@ -557,6 +558,27 @@ static int ptrace_detach(struct task_struct *child, unsigned int data)

> >  	return 0;

> >  }

> >  

> > +/*

> > + * Hook to check and report for PTRACE_SYSEMU, can be called from arch

> > + * arch syscall entry code

> > + */

> > +long ptrace_syscall_enter(struct pt_regs *regs)

> > +{

> > +#ifdef TIF_SYSCALL_EMU

> > +	if (test_thread_flag(TIF_SYSCALL_EMU)) {

> > +		if (tracehook_report_syscall_entry(regs))

> > +			/*

> > +			 * We can ignore the return code here as we need

> > +			 * return -1 always for syscall emulation irrespective

> > +			 * of whether the tracehook report fails or succeed.

> > +			 */

> > +			;

> 

> This is problematic as it causes build errors with -Werror=empty-body,

> see https://lore.kernel.org/lkml/20181218205305.26647-1-malat@debian.org/

> 


Thanks for the pointer, will update.

--
Regards,
Sudeep
Sudeep Holla March 18, 2019, 2:57 p.m. UTC | #4
On Mon, Mar 18, 2019 at 05:41:15PM +0300, Dmitry V. Levin wrote:
> On Mon, Mar 18, 2019 at 10:49:21AM +0000, Sudeep Holla wrote:

> > Currently each architecture handles PTRACE_SYSEMU in very similar way.

> > It's completely arch independent and can be handled in the code helping

> > to consolidate PTRACE_SYSEMU handling.

> > 

> > Let's introduce a hook 'ptrace_syscall_enter' that arch specific syscall

> > entry code can call.

> 

> Sorry if I'm late for the party, but the new name looks confusing.

> If all it does is related to TIF_SYSCALL_EMU, why does it have a generic

> name 'ptrace_syscall_enter' without any hint of being specific to

> TIF_SYSCALL_EMU?

> 


Not at all late. Infact Haibo Xu pointed that out, I updated but somehow
missed to commit and lost those changes. I will rename as
ptrace_sysemu_syscall_enter

--
Regards,
Sudeep
diff mbox series

Patch

diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index edb9b040c94c..e30f51e3363e 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -407,6 +407,7 @@  static inline void user_single_step_report(struct pt_regs *regs)
 #define current_user_stack_pointer() user_stack_pointer(current_pt_regs())
 #endif
 
+extern long ptrace_syscall_enter(struct pt_regs *regs);
 extern int task_current_syscall(struct task_struct *target, long *callno,
 				unsigned long args[6], unsigned int maxargs,
 				unsigned long *sp, unsigned long *pc);
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 4fa3b7f4c3c7..c9c505c483df 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -29,6 +29,7 @@ 
 #include <linux/hw_breakpoint.h>
 #include <linux/cn_proc.h>
 #include <linux/compat.h>
+#include <linux/tracehook.h>
 
 /*
  * Access another process' address space via ptrace.
@@ -557,6 +558,27 @@  static int ptrace_detach(struct task_struct *child, unsigned int data)
 	return 0;
 }
 
+/*
+ * Hook to check and report for PTRACE_SYSEMU, can be called from arch
+ * arch syscall entry code
+ */
+long ptrace_syscall_enter(struct pt_regs *regs)
+{
+#ifdef TIF_SYSCALL_EMU
+	if (test_thread_flag(TIF_SYSCALL_EMU)) {
+		if (tracehook_report_syscall_entry(regs))
+			/*
+			 * We can ignore the return code here as we need
+			 * return -1 always for syscall emulation irrespective
+			 * of whether the tracehook report fails or succeed.
+			 */
+			;
+		return -1L;
+	}
+#endif
+	return 0;
+}
+
 /*
  * Detach all tasks we were using ptrace on. Called with tasklist held
  * for writing.