[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[Andrew Gabbasov]

sys_exit tracepoint expects the syscall return value as a second
argument, rather than syscall number.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
---
 arch/arm/kernel/ptrace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 739db3a..4206da7 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -956,7 +956,7 @@ asmlinkage int syscall_trace_exit(struct pt_regs *regs, int scno)
 {
 	scno = ptrace_syscall_trace(regs, scno, PTRACE_SYSCALL_EXIT);
 	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
-		trace_sys_exit(regs, scno);
+		trace_sys_exit(regs, regs->ARM_r0);
 	audit_syscall_exit(regs);
 	return scno;
 }
-- 
1.7.10.4

[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[Will Deacon]

On Sat, Dec 01, 2012 at 01:38:54PM +0000, Andrew Gabbasov wrote:
> sys_exit tracepoint expects the syscall return value as a second
> argument, rather than syscall number.
> 
> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> ---
>  arch/arm/kernel/ptrace.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
> index 739db3a..4206da7 100644
> --- a/arch/arm/kernel/ptrace.c
> +++ b/arch/arm/kernel/ptrace.c
> @@ -956,7 +956,7 @@ asmlinkage int syscall_trace_exit(struct pt_regs *regs, int scno)
>  {
>  	scno = ptrace_syscall_trace(regs, scno, PTRACE_SYSCALL_EXIT);
>  	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
> -		trace_sys_exit(regs, scno);
> +		trace_sys_exit(regs, regs->ARM_r0);
>  	audit_syscall_exit(regs);
>  	return scno;
>  }

It might be worth stashing the return value into a local variable prior to
the ptrace_syscall_trace invocation, just in case a debugger decides to
rewrite the child's registers.

Will

[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[Russell King - ARM Linux]

On Mon, Dec 03, 2012 at 01:37:36PM +0000, Will Deacon wrote:
> It might be worth stashing the return value into a local variable prior to
> the ptrace_syscall_trace invocation, just in case a debugger decides to
> rewrite the child's registers.

And if so, ensure that there's a comment there explaining it, otherwise
the reasoning will get lost over time.

[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[Gabbasov, Andrew]

> On Sat, Dec 01, 2012 at 01:38:54PM +0000, Andrew Gabbasov wrote:
> > sys_exit tracepoint expects the syscall return value as a second
> > argument, rather than syscall number.
> >
> > Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> > ---
> >  arch/arm/kernel/ptrace.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
> > index 739db3a..4206da7 100644
> > --- a/arch/arm/kernel/ptrace.c
> > +++ b/arch/arm/kernel/ptrace.c
> > @@ -956,7 +956,7 @@ asmlinkage int syscall_trace_exit(struct pt_regs *regs, int scno)
> >  {
> >       scno = ptrace_syscall_trace(regs, scno, PTRACE_SYSCALL_EXIT);
> >       if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
> > -             trace_sys_exit(regs, scno);
> > +             trace_sys_exit(regs, regs->ARM_r0);
> >       audit_syscall_exit(regs);
> >       return scno;
> >  }
> 
> It might be worth stashing the return value into a local variable prior to
> the ptrace_syscall_trace invocation, just in case a debugger decides to
> rewrite the child's registers.
> 
> Will

The other registers in "regs" structure are not saved around ptrace_syscall_trace,
and both audit_syscall_exit and trace_sys_exit get the register values, potentially
changed by a debugger. Does it make sense to save the isolated return value
for trace_sys_exit call only and not to save other registers, passed, for example,
to audit_syscall_exit function that takes the return value from regs structure?
Isn't it a reasonable assumption that a debugger will preserve important
register values (or intentionally change them for some purpose) in case
of syscall_exit, as we rely on this for syscall_enter case?

Thanks,
Andrew

[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[Russell King - ARM Linux]

On Tue, Dec 04, 2012 at 09:48:28AM +0000, Gabbasov, Andrew wrote:
> The other registers in "regs" structure are not saved around ptrace_syscall_trace,
> and both audit_syscall_exit and trace_sys_exit get the register values, potentially
> changed by a debugger. Does it make sense to save the isolated return value
> for trace_sys_exit call only and not to save other registers, passed, for example,
> to audit_syscall_exit function that takes the return value from regs structure?
> Isn't it a reasonable assumption that a debugger will preserve important
> register values (or intentionally change them for some purpose) in case
> of syscall_exit, as we rely on this for syscall_enter case?

Actually, why are we doing things in a different order to x86?  If we
assume that x86 has this stuff correctly ordered, then shouldn't we
be following the sequence presented there?

x86 on entry does:
- secure_computing
- tracehook_report_syscall_entry
- trace_sys_enter
- audit_syscall_entry
and on exit:
- audit_syscall_exit
- trace_sys_exit
- tracehook_report_syscall_exit

We appear to be doing the _exact_ reverse ordering in our syscall exit
path - why is that?

[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[Will Deacon]

On Tue, Dec 04, 2012 at 10:38:09AM +0000, Russell King - ARM Linux wrote:
> On Tue, Dec 04, 2012 at 09:48:28AM +0000, Gabbasov, Andrew wrote:
> > The other registers in "regs" structure are not saved around ptrace_syscall_trace,
> > and both audit_syscall_exit and trace_sys_exit get the register values, potentially
> > changed by a debugger. Does it make sense to save the isolated return value
> > for trace_sys_exit call only and not to save other registers, passed, for example,
> > to audit_syscall_exit function that takes the return value from regs structure?
> > Isn't it a reasonable assumption that a debugger will preserve important
> > register values (or intentionally change them for some purpose) in case
> > of syscall_exit, as we rely on this for syscall_enter case?
> 
> Actually, why are we doing things in a different order to x86?  If we
> assume that x86 has this stuff correctly ordered, then shouldn't we
> be following the sequence presented there?
> 
> x86 on entry does:
> - secure_computing
> - tracehook_report_syscall_entry
> - trace_sys_enter
> - audit_syscall_entry
> and on exit:
> - audit_syscall_exit
> - trace_sys_exit
> - tracehook_report_syscall_exit
> 
> We appear to be doing the _exact_ reverse ordering in our syscall exit
> path - why is that?


I've just been looking at that and the problem stems around having ->syscall
set for the current thread before calling the sys_exit tracepoint. The audit
code should definitely be moved earlier.

Working on a patch...

Will

[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[Will Deacon]

On Tue, Dec 04, 2012 at 10:40:38AM +0000, Will Deacon wrote:
> On Tue, Dec 04, 2012 at 10:38:09AM +0000, Russell King - ARM Linux wrote:
> > On Tue, Dec 04, 2012 at 09:48:28AM +0000, Gabbasov, Andrew wrote:
> > > The other registers in "regs" structure are not saved around ptrace_syscall_trace,
> > > and both audit_syscall_exit and trace_sys_exit get the register values, potentially
> > > changed by a debugger. Does it make sense to save the isolated return value
> > > for trace_sys_exit call only and not to save other registers, passed, for example,
> > > to audit_syscall_exit function that takes the return value from regs structure?
> > > Isn't it a reasonable assumption that a debugger will preserve important
> > > register values (or intentionally change them for some purpose) in case
> > > of syscall_exit, as we rely on this for syscall_enter case?
> > 
> > Actually, why are we doing things in a different order to x86?  If we
> > assume that x86 has this stuff correctly ordered, then shouldn't we
> > be following the sequence presented there?
> > 
> > x86 on entry does:
> > - secure_computing
> > - tracehook_report_syscall_entry
> > - trace_sys_enter
> > - audit_syscall_entry
> > and on exit:
> > - audit_syscall_exit
> > - trace_sys_exit
> > - tracehook_report_syscall_exit
> > 
> > We appear to be doing the _exact_ reverse ordering in our syscall exit
> > path - why is that?
> 
> 
> I've just been looking at that and the problem stems around having ->syscall
> set for the current thread before calling the sys_exit tracepoint. The audit
> code should definitely be moved earlier.
> 
> Working on a patch...

Ok, how about something like the following?

Will

--->8

commit 0109b339755b29f92733c1f257911615fc231727
Author: Will Deacon <will.deacon@arm.com>
Date:   Tue Dec 4 11:47:14 2012 +0000

    ARM: syscall: rework ordering in syscall_trace_exit
    
    syscall_trace_exit is currently doing things back-to-front; invoking
    the audit hook *after* signalling the debugger, which presents an
    opportunity for the registers to be re-written by userspace in order to
    bypass auditing constaints.
    
    This patch fixes the ordering by moving the audit code first and the
    tracehook code last. On the face of it, it looks like
    current_thread_info()->syscall may be incorrect for the sys_exit
    tracepoint, but that's actually not an issue because it will have been
    set during syscall entry and cannot have changed since then.
    
    Reported-by: Andrew Gabbasov <Andrew_Gabbasov@mentor.com>
    Signed-off-by: Will Deacon <will.deacon@arm.com>

diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index 8355d4b..1dd5122 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -452,7 +452,6 @@ __sys_trace:
 
 __sys_trace_return:
 	str	r0, [sp, #S_R0 + S_OFF]!	@ save returned r0
-	mov	r1, scno
 	mov	r0, sp
 	bl	syscall_trace_exit
 	b	ret_slow_syscall
diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index 518536d..0e9c779 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -957,17 +957,22 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
 	return scno;
 }
 
-asmlinkage int syscall_trace_exit(struct pt_regs *regs, int scno)
+asmlinkage void syscall_trace_exit(struct pt_regs *regs)
 {
-	current_thread_info()->syscall = scno;
-
-	if (test_thread_flag(TIF_SYSCALL_TRACE))
-		scno = tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
+	/*
+	 * Audit the syscall before anything else, as a debugger may
+	 * come in and change the current registers.
+	 */
+	audit_syscall_exit(regs);
 
+	/*
+	 * Note that we haven't updated the ->syscall field for the
+	 * current thread. This isn't a problem because it will have
+	 * been set on syscall entry and there hasn't been an opportunity
+	 * for a PTRACE_SET_SYSCALL since then.
+	 */
 	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
-		trace_sys_exit(regs, scno);
+		trace_sys_exit(regs, regs_return_value(regs));
 
-	audit_syscall_exit(regs);
-
-	return scno;
+	tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
 }

[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[Gabbasov, Andrew]

> On Tue, Dec 04, 2012 at 10:40:38AM +0000, Will Deacon wrote:
> > On Tue, Dec 04, 2012 at 10:38:09AM +0000, Russell King - ARM Linux wrote:
> > > On Tue, Dec 04, 2012 at 09:48:28AM +0000, Gabbasov, Andrew wrote:
> > > > The other registers in "regs" structure are not saved around ptrace_syscall_trace,
> > > > and both audit_syscall_exit and trace_sys_exit get the register values, potentially
> > > > changed by a debugger. Does it make sense to save the isolated return value
> > > > for trace_sys_exit call only and not to save other registers, passed, for example,
> > > > to audit_syscall_exit function that takes the return value from regs structure?
> > > > Isn't it a reasonable assumption that a debugger will preserve important
> > > > register values (or intentionally change them for some purpose) in case
> > > > of syscall_exit, as we rely on this for syscall_enter case?
> > >
> > > Actually, why are we doing things in a different order to x86?  If we
> > > assume that x86 has this stuff correctly ordered, then shouldn't we
> > > be following the sequence presented there?
> > >
> > > x86 on entry does:
> > > - secure_computing
> > > - tracehook_report_syscall_entry
> > > - trace_sys_enter
> > > - audit_syscall_entry
> > > and on exit:
> > > - audit_syscall_exit
> > > - trace_sys_exit
> > > - tracehook_report_syscall_exit
> > >
> > > We appear to be doing the _exact_ reverse ordering in our syscall exit
> > > path - why is that?
> >
> >
> > I've just been looking at that and the problem stems around having ->syscall
> > set for the current thread before calling the sys_exit tracepoint. The audit
> > code should definitely be moved earlier.
> >
> > Working on a patch...
> 
> Ok, how about something like the following?
> 
> Will
> 
> --->8
> 
> commit 0109b339755b29f92733c1f257911615fc231727
> Author: Will Deacon <will.deacon@arm.com>
> Date:   Tue Dec 4 11:47:14 2012 +0000
> 
>     ARM: syscall: rework ordering in syscall_trace_exit
> 
>    syscall_trace_exit is currently doing things back-to-front; invoking
>     the audit hook *after* signalling the debugger, which presents an
>     opportunity for the registers to be re-written by userspace in order to
>     bypass auditing constaints.
> 
>     This patch fixes the ordering by moving the audit code first and the
>     tracehook code last. On the face of it, it looks like
>     current_thread_info()->syscall may be incorrect for the sys_exit
>     tracepoint, but that's actually not an issue because it will have been
>     set during syscall entry and cannot have changed since then.
> 
>     Reported-by: Andrew Gabbasov <Andrew_Gabbasov@mentor.com>
>     Signed-off-by: Will Deacon <will.deacon@arm.com>
> 
> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
> index 8355d4b..1dd5122 100644
> --- a/arch/arm/kernel/entry-common.S
> +++ b/arch/arm/kernel/entry-common.S
> @@ -452,7 +452,6 @@ __sys_trace:
> 
>  __sys_trace_return:
>         str     r0, [sp, #S_R0 + S_OFF]!        @ save returned r0
> -       mov     r1, scno
>         mov     r0, sp
>         bl      syscall_trace_exit
>         b       ret_slow_syscall
> diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
> index 518536d..0e9c779 100644
> --- a/arch/arm/kernel/ptrace.c
> +++ b/arch/arm/kernel/ptrace.c
> @@ -957,17 +957,22 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
>         return scno;
>  }
> 
> -asmlinkage int syscall_trace_exit(struct pt_regs *regs, int scno)
> +asmlinkage void syscall_trace_exit(struct pt_regs *regs)
>  {
> -       current_thread_info()->syscall = scno;
> -
> -       if (test_thread_flag(TIF_SYSCALL_TRACE))
> -               scno = tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
> +       /*
> +        * Audit the syscall before anything else, as a debugger may
> +        * come in and change the current registers.
> +        */
> +       audit_syscall_exit(regs);
> 
> +       /*
> +        * Note that we haven't updated the ->syscall field for the
> +        * current thread. This isn't a problem because it will have
> +        * been set on syscall entry and there hasn't been an opportunity
> +        * for a PTRACE_SET_SYSCALL since then.
> +        */
>         if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
> -               trace_sys_exit(regs, scno);
> +               trace_sys_exit(regs, regs_return_value(regs));
> 
> -       audit_syscall_exit(regs);
> -
> -       return scno;
> +       tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
>  }
> 

Hi Will,

Thank you for the patch, I was thinking about something like this too, but was in a doubt
that reverse order could be for a purpose ;-)

Minor question: what is the version this patch is against? The latest version in Linus'es tree
does not have "current_thread_info()->syscall = scno;" at the beginning of
syscall_trace_exit. Or is it just a "proof of concept" patch and will be modified before
merging to the source?

Thanks,
Andrew

[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[Will Deacon]

On Thu, Dec 06, 2012 at 09:53:58AM +0000, Gabbasov, Andrew wrote:
> Hi Will,

Hello Andrew,

> Thank you for the patch, I was thinking about something like this too, but was in a doubt
> that reverse order could be for a purpose ;-)

Nope, it think it was an oversight when the entry path was cleaned up.

> Minor question: what is the version this patch is against? The latest version in Linus'es tree
> does not have "current_thread_info()->syscall = scno;" at the beginning of
> syscall_trace_exit. Or is it just a "proof of concept" patch and will be modified before
> merging to the source?

It's based on what Russell has queued for 3.8 (there is some seccomp stuff
in there). If you're happy with the patch, I can stick it in the patch
system once I've given it some more testing.

Will

[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[Gabbasov, Andrew]

> On Thu, Dec 06, 2012 at 09:53:58AM +0000, Gabbasov, Andrew wrote:
> > Hi Will,
> 
> Hello Andrew,
> 
> > Thank you for the patch, I was thinking about something like this too, but was in a doubt
> > that reverse order could be for a purpose ;-)
> 
> Nope, it think it was an oversight when the entry path was cleaned up.
> 
> > Minor question: what is the version this patch is against? The latest version in Linus'es tree
> > does not have "current_thread_info()->syscall = scno;" at the beginning of
> > syscall_trace_exit. Or is it just a "proof of concept" patch and will be modified before
> > merging to the source?
> 
> It's based on what Russell has queued for 3.8 (there is some seccomp stuff
> in there). If you're happy with the patch, I can stick it in the patch
> system once I've given it some more testing.
> 
> Will

Yes, the patch looks good to me.

Thanks.
Andrew

[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[Mark Rutland]

On Tue, Dec 04, 2012 at 11:55:26AM +0000, Will Deacon wrote:
> On Tue, Dec 04, 2012 at 10:40:38AM +0000, Will Deacon wrote:
> > On Tue, Dec 04, 2012 at 10:38:09AM +0000, Russell King - ARM Linux wrote:
> > > On Tue, Dec 04, 2012 at 09:48:28AM +0000, Gabbasov, Andrew wrote:
> > > > The other registers in "regs" structure are not saved around ptrace_syscall_trace,
> > > > and both audit_syscall_exit and trace_sys_exit get the register values, potentially
> > > > changed by a debugger. Does it make sense to save the isolated return value
> > > > for trace_sys_exit call only and not to save other registers, passed, for example,
> > > > to audit_syscall_exit function that takes the return value from regs structure?
> > > > Isn't it a reasonable assumption that a debugger will preserve important
> > > > register values (or intentionally change them for some purpose) in case
> > > > of syscall_exit, as we rely on this for syscall_enter case?
> > > 
> > > Actually, why are we doing things in a different order to x86?  If we
> > > assume that x86 has this stuff correctly ordered, then shouldn't we
> > > be following the sequence presented there?
> > > 
> > > x86 on entry does:
> > > - secure_computing
> > > - tracehook_report_syscall_entry
> > > - trace_sys_enter
> > > - audit_syscall_entry
> > > and on exit:
> > > - audit_syscall_exit
> > > - trace_sys_exit
> > > - tracehook_report_syscall_exit
> > > 
> > > We appear to be doing the _exact_ reverse ordering in our syscall exit
> > > path - why is that?
> > 
> > 
> > I've just been looking at that and the problem stems around having ->syscall
> > set for the current thread before calling the sys_exit tracepoint. The audit
> > code should definitely be moved earlier.
> > 
> > Working on a patch...
> 
> Ok, how about something like the following?
> 
> Will
> 
> --->8
> 
> commit 0109b339755b29f92733c1f257911615fc231727
> Author: Will Deacon <will.deacon@arm.com>
> Date:   Tue Dec 4 11:47:14 2012 +0000
> 
>     ARM: syscall: rework ordering in syscall_trace_exit
>     
>     syscall_trace_exit is currently doing things back-to-front; invoking
>     the audit hook *after* signalling the debugger, which presents an
>     opportunity for the registers to be re-written by userspace in order to
>     bypass auditing constaints.
>     
>     This patch fixes the ordering by moving the audit code first and the
>     tracehook code last. On the face of it, it looks like
>     current_thread_info()->syscall may be incorrect for the sys_exit
>     tracepoint, but that's actually not an issue because it will have been
>     set during syscall entry and cannot have changed since then.
>     
>     Reported-by: Andrew Gabbasov <Andrew_Gabbasov@mentor.com>
>     Signed-off-by: Will Deacon <will.deacon@arm.com>

My board locks up upon entering userspace with this applied.

> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
> index 8355d4b..1dd5122 100644
> --- a/arch/arm/kernel/entry-common.S
> +++ b/arch/arm/kernel/entry-common.S
> @@ -452,7 +452,6 @@ __sys_trace:
>  
>  __sys_trace_return:
>  	str	r0, [sp, #S_R0 + S_OFF]!	@ save returned r0
> -	mov	r1, scno
>  	mov	r0, sp
>  	bl	syscall_trace_exit
>  	b	ret_slow_syscall
> diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
> index 518536d..0e9c779 100644
> --- a/arch/arm/kernel/ptrace.c
> +++ b/arch/arm/kernel/ptrace.c
> @@ -957,17 +957,22 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs, int scno)
>  	return scno;
>  }
>  
> -asmlinkage int syscall_trace_exit(struct pt_regs *regs, int scno)
> +asmlinkage void syscall_trace_exit(struct pt_regs *regs)
>  {
> -	current_thread_info()->syscall = scno;
> -
> -	if (test_thread_flag(TIF_SYSCALL_TRACE))
> -		scno = tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);

I see here we used to test TIF_SYSCALL_TRACE...

> +	/*
> +	 * Audit the syscall before anything else, as a debugger may
> +	 * come in and change the current registers.
> +	 */
> +	audit_syscall_exit(regs);
>  
> +	/*
> +	 * Note that we haven't updated the ->syscall field for the
> +	 * current thread. This isn't a problem because it will have
> +	 * been set on syscall entry and there hasn't been an opportunity
> +	 * for a PTRACE_SET_SYSCALL since then.
> +	 */
>  	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
> -		trace_sys_exit(regs, scno);
> +		trace_sys_exit(regs, regs_return_value(regs));
>  
> -	audit_syscall_exit(regs);
> -
> -	return scno;
> +	tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);

And here we don't. Restoring the test makes boot complete, and audit and
tracing seem to be happy afterwards.

With the test restored,
Tested-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

[PATCH] ARM: pass syscall return value to sys_exit tracepoint

[Will Deacon]

On Fri, Dec 07, 2012 at 10:45:40AM +0000, Mark Rutland wrote:
> On Tue, Dec 04, 2012 at 11:55:26AM +0000, Will Deacon wrote:
> > +asmlinkage void syscall_trace_exit(struct pt_regs *regs)
> >  {
> > -	current_thread_info()->syscall = scno;
> > -
> > -	if (test_thread_flag(TIF_SYSCALL_TRACE))
> > -		scno = tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
> 
> I see here we used to test TIF_SYSCALL_TRACE...
> 
> > +	/*
> > +	 * Audit the syscall before anything else, as a debugger may
> > +	 * come in and change the current registers.
> > +	 */
> > +	audit_syscall_exit(regs);
> >  
> > +	/*
> > +	 * Note that we haven't updated the ->syscall field for the
> > +	 * current thread. This isn't a problem because it will have
> > +	 * been set on syscall entry and there hasn't been an opportunity
> > +	 * for a PTRACE_SET_SYSCALL since then.
> > +	 */
> >  	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
> > -		trace_sys_exit(regs, scno);
> > +		trace_sys_exit(regs, regs_return_value(regs));
> >  
> > -	audit_syscall_exit(regs);
> > -
> > -	return scno;
> > +	tracehook_report_syscall(regs, PTRACE_SYSCALL_EXIT);
> 
> And here we don't. Restoring the test makes boot complete, and audit and
> tracing seem to be happy afterwards.
> 
> With the test restored,
> Tested-by: Mark Rutland <mark.rutland@arm.com>

Cheers Mark, I'll fix this oversight and stick it in the patch system with
your tag.

Will