Re: [PATCH v5 2/4] arm64: split syscall_trace() into separate functions for enter/exit

2014-04-28 Thread AKASHI Takahiro

On 04/16/2014 10:27 PM, Will Deacon wrote:

Hi Akashi,

On Sat, Mar 15, 2014 at 05:39:06AM +, AKASHI Takahiro wrote:

As done in arm, this change makes it easy to confirm we invoke syscall
related hooks, including syscall tracepoint, audit and seccomp which would
be implemented later, in correct order. That is, undoing operations in the
opposite order on exit that they were done on entry.

Signed-off-by: AKASHI Takahiro 


[...]


+static void tracehook_report_syscall(struct pt_regs *regs,
+enum ptrace_syscall_dir dir)
  {
+   int scrach;


s/scrach/scratch/


I will fix it.


Although, I'd rather have a variable with a more meaningful name. How about
regno?


OK, I will use regno in the next revision, which I will submit soon.


With that,

   Acked-by: Will Deacon 


Thank you so much,
-Takahiro AKASHI


Cheers,

Will


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v5 2/4] arm64: split syscall_trace() into separate functions for enter/exit

2014-04-28 Thread AKASHI Takahiro

On 04/16/2014 10:27 PM, Will Deacon wrote:

Hi Akashi,

On Sat, Mar 15, 2014 at 05:39:06AM +, AKASHI Takahiro wrote:

As done in arm, this change makes it easy to confirm we invoke syscall
related hooks, including syscall tracepoint, audit and seccomp which would
be implemented later, in correct order. That is, undoing operations in the
opposite order on exit that they were done on entry.

Signed-off-by: AKASHI Takahiro takahiro.aka...@linaro.org


[...]


+static void tracehook_report_syscall(struct pt_regs *regs,
+enum ptrace_syscall_dir dir)
  {
+   int scrach;


s/scrach/scratch/


I will fix it.


Although, I'd rather have a variable with a more meaningful name. How about
regno?


OK, I will use regno in the next revision, which I will submit soon.


With that,

   Acked-by: Will Deacon will.dea...@arm.com


Thank you so much,
-Takahiro AKASHI


Cheers,

Will


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v5 2/4] arm64: split syscall_trace() into separate functions for enter/exit

2014-04-16 Thread Will Deacon
Hi Akashi,

On Sat, Mar 15, 2014 at 05:39:06AM +, AKASHI Takahiro wrote:
> As done in arm, this change makes it easy to confirm we invoke syscall
> related hooks, including syscall tracepoint, audit and seccomp which would
> be implemented later, in correct order. That is, undoing operations in the
> opposite order on exit that they were done on entry.
> 
> Signed-off-by: AKASHI Takahiro 

[...]

> +static void tracehook_report_syscall(struct pt_regs *regs,
> +  enum ptrace_syscall_dir dir)
>  {
> + int scrach;

s/scrach/scratch/

Although, I'd rather have a variable with a more meaningful name. How about
regno?

With that,

  Acked-by: Will Deacon 

Cheers,

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v5 2/4] arm64: split syscall_trace() into separate functions for enter/exit

2014-04-16 Thread Will Deacon
Hi Akashi,

On Sat, Mar 15, 2014 at 05:39:06AM +, AKASHI Takahiro wrote:
 As done in arm, this change makes it easy to confirm we invoke syscall
 related hooks, including syscall tracepoint, audit and seccomp which would
 be implemented later, in correct order. That is, undoing operations in the
 opposite order on exit that they were done on entry.
 
 Signed-off-by: AKASHI Takahiro takahiro.aka...@linaro.org

[...]

 +static void tracehook_report_syscall(struct pt_regs *regs,
 +  enum ptrace_syscall_dir dir)
  {
 + int scrach;

s/scrach/scratch/

Although, I'd rather have a variable with a more meaningful name. How about
regno?

With that,

  Acked-by: Will Deacon will.dea...@arm.com

Cheers,

Will
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v5 2/4] arm64: split syscall_trace() into separate functions for enter/exit

2014-03-16 Thread Richard Guy Briggs
On 14/03/15, AKASHI Takahiro wrote:
> As done in arm, this change makes it easy to confirm we invoke syscall
> related hooks, including syscall tracepoint, audit and seccomp which would
> be implemented later, in correct order. That is, undoing operations in the
> opposite order on exit that they were done on entry.
> 
> Signed-off-by: AKASHI Takahiro 

Minor variable mis-spelling of "scratch" noted below, but other than
that:

Acked-by: Richard Guy Briggs 

> ---
>  arch/arm64/kernel/entry.S  | 10 --
>  arch/arm64/kernel/ptrace.c | 50 
> +++---
>  2 files changed, 33 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index f9f2cae..00d6eb9 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -649,9 +649,8 @@ ENDPROC(el0_svc)
>* switches, and waiting for our parent to respond.
>*/
>  __sys_trace:
> - mov x1, sp
> - mov w0, #0  // trace entry
> - bl  syscall_trace
> + mov x0, sp
> + bl  syscall_trace_enter
>   adr lr, __sys_trace_return  // return address
>   uxtwscno, w0// syscall number (possibly new)
>   mov x1, sp  // pointer to regs
> @@ -666,9 +665,8 @@ __sys_trace:
>  
>  __sys_trace_return:
>   str x0, [sp]// save returned x0
> - mov x1, sp
> - mov w0, #1  // trace exit
> - bl  syscall_trace
> + mov x0, sp
> + bl  syscall_trace_exit
>   b   ret_to_user
>  
>  /*
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 6a8928b..f606276 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -1058,35 +1058,43 @@ long arch_ptrace(struct task_struct *child, long 
> request,
>   return ptrace_request(child, request, addr, data);
>  }
>  
> -asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
> +enum ptrace_syscall_dir {
> + PTRACE_SYSCALL_ENTER = 0,
> + PTRACE_SYSCALL_EXIT,
> +};
> +
> +static void tracehook_report_syscall(struct pt_regs *regs,
> +  enum ptrace_syscall_dir dir)
>  {
> + int scrach;

"scratch"

>   unsigned long saved_reg;
>  
> - if (!test_thread_flag(TIF_SYSCALL_TRACE))
> - return regs->syscallno;
> -
> - if (is_compat_task()) {
> - /* AArch32 uses ip (r12) for scratch */
> - saved_reg = regs->regs[12];
> - regs->regs[12] = dir;
> - } else {
> - /*
> -  * Save X7. X7 is used to denote syscall entry/exit:
> -  *   X7 = 0 -> entry, = 1 -> exit
> -  */
> - saved_reg = regs->regs[7];
> - regs->regs[7] = dir;
> - }
> + /*
> +  * A scrach register (ip(r12) on AArch32, x7 on AArch64) is
> +  * used to denote syscall entry/exit:
> +  */
> + scrach = (is_compat_task() ? 12 : 7);
> + saved_reg = regs->regs[scrach];
> + regs->regs[scrach] = dir;
>  
> - if (dir)
> + if (dir == PTRACE_SYSCALL_EXIT)
>   tracehook_report_syscall_exit(regs, 0);
>   else if (tracehook_report_syscall_entry(regs))
>   regs->syscallno = ~0UL;
>  
> - if (is_compat_task())
> - regs->regs[12] = saved_reg;
> - else
> - regs->regs[7] = saved_reg;
> + regs->regs[scrach] = saved_reg;
> +}
> +
> +asmlinkage int syscall_trace_enter(struct pt_regs *regs)
> +{
> + if (test_thread_flag(TIF_SYSCALL_TRACE))
> + tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
>  
>   return regs->syscallno;
>  }
> +
> +asmlinkage void syscall_trace_exit(struct pt_regs *regs)
> +{
> + if (test_thread_flag(TIF_SYSCALL_TRACE))
> + tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
> +}
> -- 
> 1.8.3.2

- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v5 2/4] arm64: split syscall_trace() into separate functions for enter/exit

2014-03-16 Thread Richard Guy Briggs
On 14/03/15, AKASHI Takahiro wrote:
 As done in arm, this change makes it easy to confirm we invoke syscall
 related hooks, including syscall tracepoint, audit and seccomp which would
 be implemented later, in correct order. That is, undoing operations in the
 opposite order on exit that they were done on entry.
 
 Signed-off-by: AKASHI Takahiro takahiro.aka...@linaro.org

Minor variable mis-spelling of scratch noted below, but other than
that:

Acked-by: Richard Guy Briggs r...@redhat.com

 ---
  arch/arm64/kernel/entry.S  | 10 --
  arch/arm64/kernel/ptrace.c | 50 
 +++---
  2 files changed, 33 insertions(+), 27 deletions(-)
 
 diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
 index f9f2cae..00d6eb9 100644
 --- a/arch/arm64/kernel/entry.S
 +++ b/arch/arm64/kernel/entry.S
 @@ -649,9 +649,8 @@ ENDPROC(el0_svc)
* switches, and waiting for our parent to respond.
*/
  __sys_trace:
 - mov x1, sp
 - mov w0, #0  // trace entry
 - bl  syscall_trace
 + mov x0, sp
 + bl  syscall_trace_enter
   adr lr, __sys_trace_return  // return address
   uxtwscno, w0// syscall number (possibly new)
   mov x1, sp  // pointer to regs
 @@ -666,9 +665,8 @@ __sys_trace:
  
  __sys_trace_return:
   str x0, [sp]// save returned x0
 - mov x1, sp
 - mov w0, #1  // trace exit
 - bl  syscall_trace
 + mov x0, sp
 + bl  syscall_trace_exit
   b   ret_to_user
  
  /*
 diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
 index 6a8928b..f606276 100644
 --- a/arch/arm64/kernel/ptrace.c
 +++ b/arch/arm64/kernel/ptrace.c
 @@ -1058,35 +1058,43 @@ long arch_ptrace(struct task_struct *child, long 
 request,
   return ptrace_request(child, request, addr, data);
  }
  
 -asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
 +enum ptrace_syscall_dir {
 + PTRACE_SYSCALL_ENTER = 0,
 + PTRACE_SYSCALL_EXIT,
 +};
 +
 +static void tracehook_report_syscall(struct pt_regs *regs,
 +  enum ptrace_syscall_dir dir)
  {
 + int scrach;

scratch

   unsigned long saved_reg;
  
 - if (!test_thread_flag(TIF_SYSCALL_TRACE))
 - return regs-syscallno;
 -
 - if (is_compat_task()) {
 - /* AArch32 uses ip (r12) for scratch */
 - saved_reg = regs-regs[12];
 - regs-regs[12] = dir;
 - } else {
 - /*
 -  * Save X7. X7 is used to denote syscall entry/exit:
 -  *   X7 = 0 - entry, = 1 - exit
 -  */
 - saved_reg = regs-regs[7];
 - regs-regs[7] = dir;
 - }
 + /*
 +  * A scrach register (ip(r12) on AArch32, x7 on AArch64) is
 +  * used to denote syscall entry/exit:
 +  */
 + scrach = (is_compat_task() ? 12 : 7);
 + saved_reg = regs-regs[scrach];
 + regs-regs[scrach] = dir;
  
 - if (dir)
 + if (dir == PTRACE_SYSCALL_EXIT)
   tracehook_report_syscall_exit(regs, 0);
   else if (tracehook_report_syscall_entry(regs))
   regs-syscallno = ~0UL;
  
 - if (is_compat_task())
 - regs-regs[12] = saved_reg;
 - else
 - regs-regs[7] = saved_reg;
 + regs-regs[scrach] = saved_reg;
 +}
 +
 +asmlinkage int syscall_trace_enter(struct pt_regs *regs)
 +{
 + if (test_thread_flag(TIF_SYSCALL_TRACE))
 + tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
  
   return regs-syscallno;
  }
 +
 +asmlinkage void syscall_trace_exit(struct pt_regs *regs)
 +{
 + if (test_thread_flag(TIF_SYSCALL_TRACE))
 + tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
 +}
 -- 
 1.8.3.2

- RGB

--
Richard Guy Briggs rbri...@redhat.com
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v5 2/4] arm64: split syscall_trace() into separate functions for enter/exit

2014-03-14 Thread AKASHI Takahiro
As done in arm, this change makes it easy to confirm we invoke syscall
related hooks, including syscall tracepoint, audit and seccomp which would
be implemented later, in correct order. That is, undoing operations in the
opposite order on exit that they were done on entry.

Signed-off-by: AKASHI Takahiro 
---
 arch/arm64/kernel/entry.S  | 10 --
 arch/arm64/kernel/ptrace.c | 50 +++---
 2 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index f9f2cae..00d6eb9 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -649,9 +649,8 @@ ENDPROC(el0_svc)
 * switches, and waiting for our parent to respond.
 */
 __sys_trace:
-   mov x1, sp
-   mov w0, #0  // trace entry
-   bl  syscall_trace
+   mov x0, sp
+   bl  syscall_trace_enter
adr lr, __sys_trace_return  // return address
uxtwscno, w0// syscall number (possibly new)
mov x1, sp  // pointer to regs
@@ -666,9 +665,8 @@ __sys_trace:
 
 __sys_trace_return:
str x0, [sp]// save returned x0
-   mov x1, sp
-   mov w0, #1  // trace exit
-   bl  syscall_trace
+   mov x0, sp
+   bl  syscall_trace_exit
b   ret_to_user
 
 /*
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 6a8928b..f606276 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -1058,35 +1058,43 @@ long arch_ptrace(struct task_struct *child, long 
request,
return ptrace_request(child, request, addr, data);
 }
 
-asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
+enum ptrace_syscall_dir {
+   PTRACE_SYSCALL_ENTER = 0,
+   PTRACE_SYSCALL_EXIT,
+};
+
+static void tracehook_report_syscall(struct pt_regs *regs,
+enum ptrace_syscall_dir dir)
 {
+   int scrach;
unsigned long saved_reg;
 
-   if (!test_thread_flag(TIF_SYSCALL_TRACE))
-   return regs->syscallno;
-
-   if (is_compat_task()) {
-   /* AArch32 uses ip (r12) for scratch */
-   saved_reg = regs->regs[12];
-   regs->regs[12] = dir;
-   } else {
-   /*
-* Save X7. X7 is used to denote syscall entry/exit:
-*   X7 = 0 -> entry, = 1 -> exit
-*/
-   saved_reg = regs->regs[7];
-   regs->regs[7] = dir;
-   }
+   /*
+* A scrach register (ip(r12) on AArch32, x7 on AArch64) is
+* used to denote syscall entry/exit:
+*/
+   scrach = (is_compat_task() ? 12 : 7);
+   saved_reg = regs->regs[scrach];
+   regs->regs[scrach] = dir;
 
-   if (dir)
+   if (dir == PTRACE_SYSCALL_EXIT)
tracehook_report_syscall_exit(regs, 0);
else if (tracehook_report_syscall_entry(regs))
regs->syscallno = ~0UL;
 
-   if (is_compat_task())
-   regs->regs[12] = saved_reg;
-   else
-   regs->regs[7] = saved_reg;
+   regs->regs[scrach] = saved_reg;
+}
+
+asmlinkage int syscall_trace_enter(struct pt_regs *regs)
+{
+   if (test_thread_flag(TIF_SYSCALL_TRACE))
+   tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
 
return regs->syscallno;
 }
+
+asmlinkage void syscall_trace_exit(struct pt_regs *regs)
+{
+   if (test_thread_flag(TIF_SYSCALL_TRACE))
+   tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
+}
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v5 2/4] arm64: split syscall_trace() into separate functions for enter/exit

2014-03-14 Thread AKASHI Takahiro
As done in arm, this change makes it easy to confirm we invoke syscall
related hooks, including syscall tracepoint, audit and seccomp which would
be implemented later, in correct order. That is, undoing operations in the
opposite order on exit that they were done on entry.

Signed-off-by: AKASHI Takahiro takahiro.aka...@linaro.org
---
 arch/arm64/kernel/entry.S  | 10 --
 arch/arm64/kernel/ptrace.c | 50 +++---
 2 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index f9f2cae..00d6eb9 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -649,9 +649,8 @@ ENDPROC(el0_svc)
 * switches, and waiting for our parent to respond.
 */
 __sys_trace:
-   mov x1, sp
-   mov w0, #0  // trace entry
-   bl  syscall_trace
+   mov x0, sp
+   bl  syscall_trace_enter
adr lr, __sys_trace_return  // return address
uxtwscno, w0// syscall number (possibly new)
mov x1, sp  // pointer to regs
@@ -666,9 +665,8 @@ __sys_trace:
 
 __sys_trace_return:
str x0, [sp]// save returned x0
-   mov x1, sp
-   mov w0, #1  // trace exit
-   bl  syscall_trace
+   mov x0, sp
+   bl  syscall_trace_exit
b   ret_to_user
 
 /*
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 6a8928b..f606276 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -1058,35 +1058,43 @@ long arch_ptrace(struct task_struct *child, long 
request,
return ptrace_request(child, request, addr, data);
 }
 
-asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
+enum ptrace_syscall_dir {
+   PTRACE_SYSCALL_ENTER = 0,
+   PTRACE_SYSCALL_EXIT,
+};
+
+static void tracehook_report_syscall(struct pt_regs *regs,
+enum ptrace_syscall_dir dir)
 {
+   int scrach;
unsigned long saved_reg;
 
-   if (!test_thread_flag(TIF_SYSCALL_TRACE))
-   return regs-syscallno;
-
-   if (is_compat_task()) {
-   /* AArch32 uses ip (r12) for scratch */
-   saved_reg = regs-regs[12];
-   regs-regs[12] = dir;
-   } else {
-   /*
-* Save X7. X7 is used to denote syscall entry/exit:
-*   X7 = 0 - entry, = 1 - exit
-*/
-   saved_reg = regs-regs[7];
-   regs-regs[7] = dir;
-   }
+   /*
+* A scrach register (ip(r12) on AArch32, x7 on AArch64) is
+* used to denote syscall entry/exit:
+*/
+   scrach = (is_compat_task() ? 12 : 7);
+   saved_reg = regs-regs[scrach];
+   regs-regs[scrach] = dir;
 
-   if (dir)
+   if (dir == PTRACE_SYSCALL_EXIT)
tracehook_report_syscall_exit(regs, 0);
else if (tracehook_report_syscall_entry(regs))
regs-syscallno = ~0UL;
 
-   if (is_compat_task())
-   regs-regs[12] = saved_reg;
-   else
-   regs-regs[7] = saved_reg;
+   regs-regs[scrach] = saved_reg;
+}
+
+asmlinkage int syscall_trace_enter(struct pt_regs *regs)
+{
+   if (test_thread_flag(TIF_SYSCALL_TRACE))
+   tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
 
return regs-syscallno;
 }
+
+asmlinkage void syscall_trace_exit(struct pt_regs *regs)
+{
+   if (test_thread_flag(TIF_SYSCALL_TRACE))
+   tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
+}
-- 
1.8.3.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/