[PATCH 0/5] powerpc tracehook

[PATCH 0/5] powerpc tracehook

[Roland McGrath]

These patches are posted for review, but you can just pull the GIT branch
if you prefer.  Patch 1/5 corrects a long-standing (minor) error in ptrace
behavior.  The others change no existing behavior, just enable new and
future features to work on the arch.

The following changes since commit 8be1a6d6c77ab4532e4476fdb8177030ef48b52c:
  Linus Torvalds (1):
        Merge branch 'for-linus' of git://git.kernel.org/.../roland/infiniband

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/frob/linux-2.6-utrace.git powerpc-tracehook

Roland McGrath (5):
      powerpc: tracehook_signal_handler
      powerpc: tracehook syscall
      powerpc: tracehook: asm/syscall.h
      powerpc: tracehook: TIF_NOTIFY_RESUME
      powerpc: tracehook: CONFIG_HAVE_ARCH_TRACEHOOK

 arch/powerpc/Kconfig              |    1 +
 arch/powerpc/kernel/entry_32.S    |   11 ++++-
 arch/powerpc/kernel/entry_64.S    |   10 +++-
 arch/powerpc/kernel/ptrace.c      |   47 ++++++++++-----------
 arch/powerpc/kernel/signal.c      |   21 ++++++++-
 include/asm-powerpc/ptrace.h      |    1 +
 include/asm-powerpc/signal.h      |    3 +-
 include/asm-powerpc/syscall.h     |   84 +++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/thread_info.h |    5 ++-
 9 files changed, 147 insertions(+), 36 deletions(-)
 create mode 100644 include/asm-powerpc/syscall.h


Thanks,
Roland

[PATCH 1/5] powerpc: tracehook_signal_handler

[Roland McGrath]

This makes the powerpc signal handling code call tracehook_signal_handler()
after a handler is set up.  This means that using PTRACE_SINGLESTEP to
enter a signal handler will report to ptrace on the first instruction of
the handler, instead of the second.  This is consistent with what x86 and
other machines do, and what users and debuggers want.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 arch/powerpc/kernel/signal.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index 7aada78..11a5c45 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -9,7 +9,7 @@
  * this archive for more details.
  */
 
-#include <linux/ptrace.h>
+#include <linux/tracehook.h>
 #include <linux/signal.h>
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -177,6 +177,12 @@ int do_signal(sigset_t *oldset, struct pt_regs *regs)
 		 * its frame, and we can clear the TLF_RESTORE_SIGMASK flag.
 		 */
 		current_thread_info()->local_flags &= ~_TLF_RESTORE_SIGMASK;
+
+		/*
+		 * Let tracing know that we've done the handler setup.
+		 */
+		tracehook_signal_handler(signr, &info, &ka, regs,
+					 test_thread_flag(TIF_SINGLESTEP));
 	}
 
 	return ret;

[PATCH 2/5] powerpc: tracehook syscall

[Roland McGrath]

This changes powerpc syscall tracing to use the new tracehook.h entry
points.  There is no change, only cleanup.

The assembly changes allow do_syscall_trace_enter() to abort the
syscall without losing the information about the original r0 value.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 arch/powerpc/kernel/entry_32.S |    7 +++++-
 arch/powerpc/kernel/entry_64.S |    7 +++++-
 arch/powerpc/kernel/ptrace.c   |   47 ++++++++++++++++++---------------------
 3 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 81c8324..0efc1e6 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -343,7 +343,12 @@ syscall_dotrace:
 	stw	r0,_TRAP(r1)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	do_syscall_trace_enter
-	lwz	r0,GPR0(r1)	/* Restore original registers */
+	/*
+	 * Restore argument registers possibly just changed.
+	 * We use the return value of do_syscall_trace_enter
+	 * for call number to look up in the table (r0).
+	 */
+	mr	r0,r3
 	lwz	r3,GPR3(r1)
 	lwz	r4,GPR4(r1)
 	lwz	r5,GPR5(r1)
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index d736924..79c089e 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -214,7 +214,12 @@ syscall_dotrace:
 	bl	.save_nvgprs
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	.do_syscall_trace_enter
-	ld	r0,GPR0(r1)	/* Restore original registers */
+	/*
+	 * Restore argument registers possibly just changed.
+	 * We use the return value of do_syscall_trace_enter
+	 * for the call number to look up in the table (r0).
+	 */
+	mr	r0,r3
 	ld	r3,GPR3(r1)
 	ld	r4,GPR4(r1)
 	ld	r5,GPR5(r1)
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index a5d0e78..3f9a942 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -22,6 +22,7 @@
 #include <linux/errno.h>
 #include <linux/ptrace.h>
 #include <linux/regset.h>
+#include <linux/tracehook.h>
 #include <linux/elf.h>
 #include <linux/user.h>
 #include <linux/security.h>
@@ -1013,31 +1014,24 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 	return ret;
 }
 
-static void do_syscall_trace(void)
+/*
+ * We must return the syscall number to actually look up in the table.
+ * This can be -1L to skip running any syscall at all.
+ */
+long do_syscall_trace_enter(struct pt_regs *regs)
 {
-	/* the 0x80 provides a way for the tracing parent to distinguish
-	   between a syscall stop and SIGTRAP delivery */
-	ptrace_notify(SIGTRAP | ((current->ptrace & PT_TRACESYSGOOD)
-				 ? 0x80 : 0));
-
-	/*
-	 * this isn't the same as continuing with a signal, but it will do
-	 * for normal use.  strace only continues with a signal if the
-	 * stopping signal is not SIGTRAP.  -brl
-	 */
-	if (current->exit_code) {
-		send_sig(current->exit_code, current, 1);
-		current->exit_code = 0;
-	}
-}
+	long ret = 0;
 
-void do_syscall_trace_enter(struct pt_regs *regs)
-{
 	secure_computing(regs->gpr[0]);
 
-	if (test_thread_flag(TIF_SYSCALL_TRACE)
-	    && (current->ptrace & PT_PTRACED))
-		do_syscall_trace();
+	if (test_thread_flag(TIF_SYSCALL_TRACE) &&
+	    tracehook_report_syscall_entry(regs))
+		/*
+		 * Tracing decided this syscall should not happen.
+		 * We'll return a bogus call number to get an ENOSYS
+		 * error, but leave the original number in regs->gpr[0].
+		 */
+		ret = -1L;
 
 	if (unlikely(current->audit_context)) {
 #ifdef CONFIG_PPC64
@@ -1055,16 +1049,19 @@ void do_syscall_trace_enter(struct pt_regs *regs)
 					    regs->gpr[5] & 0xffffffff,
 					    regs->gpr[6] & 0xffffffff);
 	}
+
+	return ret ?: regs->gpr[0];
 }
 
 void do_syscall_trace_leave(struct pt_regs *regs)
 {
+	int step;
+
 	if (unlikely(current->audit_context))
 		audit_syscall_exit((regs->ccr&0x10000000)?AUDITSC_FAILURE:AUDITSC_SUCCESS,
 				   regs->result);
 
-	if ((test_thread_flag(TIF_SYSCALL_TRACE)
-	     || test_thread_flag(TIF_SINGLESTEP))
-	    && (current->ptrace & PT_PTRACED))
-		do_syscall_trace();
+	step = test_thread_flag(TIF_SINGLESTEP);
+	if (step || test_thread_flag(TIF_SYSCALL_TRACE))
+		tracehook_report_syscall_exit(regs, step);
 }

[PATCH 3/5] powerpc: tracehook: asm/syscall.h

[Roland McGrath]

Add asm/syscall.h for powerpc with all the required entry points.
This will allow arch-independent tracing code for system calls.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 include/asm-powerpc/ptrace.h  |    1 +
 include/asm-powerpc/syscall.h |   84 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 0 deletions(-)
 create mode 100644 include/asm-powerpc/syscall.h

diff --git a/include/asm-powerpc/ptrace.h b/include/asm-powerpc/ptrace.h
index 3d6e310..734e075 100644
--- a/include/asm-powerpc/ptrace.h
+++ b/include/asm-powerpc/ptrace.h
@@ -84,6 +84,7 @@ struct pt_regs {
 #ifndef __ASSEMBLY__
 
 #define instruction_pointer(regs) ((regs)->nip)
+#define user_stack_pointer(regs) ((regs)->gpr[1])
 #define regs_return_value(regs) ((regs)->gpr[3])
 
 #ifdef CONFIG_SMP
diff --git a/include/asm-powerpc/syscall.h b/include/asm-powerpc/syscall.h
new file mode 100644
index 0000000..3ced23a
--- /dev/null
+++ b/include/asm-powerpc/syscall.h
@@ -0,0 +1,84 @@
+/*
+ * Access to user system call parameters and results
+ *
+ * Copyright (C) 2008 Red Hat, Inc.  All rights reserved.
+ *
+ * This copyrighted material is made available to anyone wishing to use,
+ * modify, copy, or redistribute it subject to the terms and conditions
+ * of the GNU General Public License v.2.
+ *
+ * See asm-generic/syscall.h for descriptions of what we must do here.
+ */
+
+#ifndef _ASM_SYSCALL_H
+#define _ASM_SYSCALL_H	1
+
+#include <linux/sched.h>
+
+static inline long syscall_get_nr(struct task_struct *task,
+				  struct pt_regs *regs)
+{
+	return regs->trap == 0xc01 ? regs->gpr[0] : -1L;
+}
+
+static inline void syscall_rollback(struct task_struct *task,
+				    struct pt_regs *regs)
+{
+	regs->gpr[3] = regs->orig_gpr3;
+}
+
+static inline long syscall_get_error(struct task_struct *task,
+				     struct pt_regs *regs)
+{
+	return (regs->ccr & 0x1000) ? -regs->gpr[3] : 0;
+}
+
+static inline long syscall_get_return_value(struct task_struct *task,
+					    struct pt_regs *regs)
+{
+	return regs->gpr[3];
+}
+
+static inline void syscall_set_return_value(struct task_struct *task,
+					    struct pt_regs *regs,
+					    int error, long val)
+{
+	if (error) {
+		regs->ccr |= 0x1000L;
+		regs->gpr[3] = -error;
+	} else {
+		regs->ccr &= ~0x1000L;
+		regs->gpr[3] = val;
+	}
+}
+
+static inline void syscall_get_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 unsigned int i, unsigned int n,
+					 unsigned long *args)
+{
+	BUG_ON(i + n > 6);
+#ifdef CONFIG_PPC64
+	if (test_tsk_thread_flag(task, TIF_32BIT)) {
+		/*
+		 * Zero-extend 32-bit argument values.  The high bits are
+		 * garbage ignored by the actual syscall dispatch.
+		 */
+		while (n-- > 0)
+			args[n] = (u32) regs->gpr[3 + i + n];
+		return;
+	}
+#endif
+	memcpy(args, &regs->gpr[3 + i], n * sizeof(args[0]));
+}
+
+static inline void syscall_set_arguments(struct task_struct *task,
+					 struct pt_regs *regs,
+					 unsigned int i, unsigned int n,
+					 const unsigned long *args)
+{
+	BUG_ON(i + n > 6);
+	memcpy(&regs->gpr[3 + i], args, n * sizeof(args[0]));
+}
+
+#endif	/* _ASM_SYSCALL_H */

[PATCH 4/5] powerpc: tracehook: TIF_NOTIFY_RESUME

[Roland McGrath]

This adds TIF_NOTIFY_RESUME support for powerpc.  When set,
we call tracehook_notify_resume() on the way to user mode.
This overloads do_signal() to do the work, but changes its
arguments to it has the TIF_* bits handy in a register and
drops the useless first argument that was always zero.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 arch/powerpc/kernel/entry_32.S    |    4 ++--
 arch/powerpc/kernel/entry_64.S    |    3 +--
 arch/powerpc/kernel/signal.c      |   13 ++++++++++++-
 include/asm-powerpc/signal.h      |    3 +--
 include/asm-powerpc/thread_info.h |    5 ++++-
 5 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 0efc1e6..3e7445e 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -1060,8 +1060,8 @@ do_user_signal:			/* r10 contains MSR_KERNEL here */
 	SAVE_NVGPRS(r1)
 	rlwinm	r3,r3,0,0,30
 	stw	r3,_TRAP(r1)
-2:	li	r3,0
-	addi	r4,r1,STACK_FRAME_OVERHEAD
+2:	addi	r3,r1,STACK_FRAME_OVERHEAD
+	mr	r4,r9
 	bl	do_signal
 	REST_NVGPRS(r1)
 	b	recheck
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 79c089e..2d802e9 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -643,8 +643,7 @@ user_work:
 	b	.ret_from_except_lite
 
 1:	bl	.save_nvgprs
-	li	r3,0
-	addi	r4,r1,STACK_FRAME_OVERHEAD
+	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	.do_signal
 	b	.ret_from_except
 
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index 11a5c45..060d2a8 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -112,7 +112,7 @@ static void check_syscall_restart(struct pt_regs *regs, struct k_sigaction *ka,
 	}
 }
 
-int do_signal(sigset_t *oldset, struct pt_regs *regs)
+static int do_signal_pending(sigset_t *oldset, struct pt_regs *regs)
 {
 	siginfo_t info;
 	int signr;
@@ -188,6 +188,17 @@ int do_signal(sigset_t *oldset, struct pt_regs *regs)
 	return ret;
 }
 
+void do_signal(struct pt_regs *regs, unsigned long thread_info_flags)
+{
+	if (thread_info_flags & _TIF_SIGPENDING)
+		do_signal_pending(NULL, regs);
+
+	if (thread_info_flags & _TIF_NOTIFY_RESUME) {
+		clear_thread_flag(TIF_NOTIFY_RESUME);
+		tracehook_notify_resume(regs);
+	}
+}
+
 long sys_sigaltstack(const stack_t __user *uss, stack_t __user *uoss,
 		unsigned long r5, unsigned long r6, unsigned long r7,
 		unsigned long r8, struct pt_regs *regs)
diff --git a/include/asm-powerpc/signal.h b/include/asm-powerpc/signal.h
index a8c7bab..a7360cd 100644
--- a/include/asm-powerpc/signal.h
+++ b/include/asm-powerpc/signal.h
@@ -122,8 +122,7 @@ typedef struct sigaltstack {
 
 #ifdef __KERNEL__
 struct pt_regs;
-extern int do_signal(sigset_t *oldset, struct pt_regs *regs);
-extern int do_signal32(sigset_t *oldset, struct pt_regs *regs);
+extern void do_signal(struct pt_regs *regs, unsigned long thread_info_flags);
 #define ptrace_signal_deliver(regs, cookie) do { } while (0)
 #endif /* __KERNEL__ */
 
diff --git a/include/asm-powerpc/thread_info.h b/include/asm-powerpc/thread_info.h
index a9db562..9665a26 100644
--- a/include/asm-powerpc/thread_info.h
+++ b/include/asm-powerpc/thread_info.h
@@ -108,6 +108,7 @@ static inline struct thread_info *current_thread_info(void)
 #define TIF_SECCOMP		10	/* secure computing */
 #define TIF_RESTOREALL		11	/* Restore all regs (implies NOERROR) */
 #define TIF_NOERROR		12	/* Force successful syscall return */
+#define TIF_NOTIFY_RESUME	13	/* callback before returning to user */
 #define TIF_FREEZE		14	/* Freezing for suspend */
 #define TIF_RUNLATCH		15	/* Is the runlatch enabled? */
 #define TIF_ABI_PENDING		16	/* 32/64 bit switch needed */
@@ -125,12 +126,14 @@ static inline struct thread_info *current_thread_info(void)
 #define _TIF_SECCOMP		(1<<TIF_SECCOMP)
 #define _TIF_RESTOREALL		(1<<TIF_RESTOREALL)
 #define _TIF_NOERROR		(1<<TIF_NOERROR)
+#define _TIF_NOTIFY_RESUME	(1<<TIF_NOTIFY_RESUME)
 #define _TIF_FREEZE		(1<<TIF_FREEZE)
 #define _TIF_RUNLATCH		(1<<TIF_RUNLATCH)
 #define _TIF_ABI_PENDING	(1<<TIF_ABI_PENDING)
 #define _TIF_SYSCALL_T_OR_A	(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SECCOMP)
 
-#define _TIF_USER_WORK_MASK	(_TIF_SIGPENDING | _TIF_NEED_RESCHED)
+#define _TIF_USER_WORK_MASK	(_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
+				 _TIF_NOTIFY_RESUME)
 #define _TIF_PERSYSCALL_MASK	(_TIF_RESTOREALL|_TIF_NOERROR)
 
 /* Bits in local_flags */

[PATCH 5/5] powerpc: tracehook: CONFIG_HAVE_ARCH_TRACEHOOK

[Roland McGrath]

The powerpc arch code has all the prerequisites, so set HAVE_ARCH_TRACEHOOK.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 arch/powerpc/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index fe88418..587da5e 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -117,6 +117,7 @@ config PPC
 	select HAVE_KPROBES
 	select HAVE_ARCH_KGDB
 	select HAVE_KRETPROBES
+	select HAVE_ARCH_TRACEHOOK
 	select HAVE_LMB
 	select HAVE_DMA_ATTRS if PPC64
 	select USE_GENERIC_SMP_HELPERS if SMP

Re: [PATCH 0/5] powerpc tracehook

[Benjamin Herrenschmidt]

On Sat, 2008-07-26 at 23:48 -0700, Roland McGrath wrote:
> These patches are posted for review, but you can just pull the GIT branch
> if you prefer.  Patch 1/5 corrects a long-standing (minor) error in ptrace
> behavior.  The others change no existing behavior, just enable new and
> future features to work on the arch.

Hi Roland !

Thanks for this. I haven't followed the story of tracehook so far, are
those patches dependent on something else or it's all already upstream
and do you think that's still 2.6.27 material ?

Cheers,
Ben.

> The following changes since commit 8be1a6d6c77ab4532e4476fdb8177030ef48b52c:
>   Linus Torvalds (1):
>         Merge branch 'for-linus' of git://git.kernel.org/.../roland/infiniband
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/frob/linux-2.6-utrace.git powerpc-tracehook
> 
> Roland McGrath (5):
>       powerpc: tracehook_signal_handler
>       powerpc: tracehook syscall
>       powerpc: tracehook: asm/syscall.h
>       powerpc: tracehook: TIF_NOTIFY_RESUME
>       powerpc: tracehook: CONFIG_HAVE_ARCH_TRACEHOOK
> 
>  arch/powerpc/Kconfig              |    1 +
>  arch/powerpc/kernel/entry_32.S    |   11 ++++-
>  arch/powerpc/kernel/entry_64.S    |   10 +++-
>  arch/powerpc/kernel/ptrace.c      |   47 ++++++++++-----------
>  arch/powerpc/kernel/signal.c      |   21 ++++++++-
>  include/asm-powerpc/ptrace.h      |    1 +
>  include/asm-powerpc/signal.h      |    3 +-
>  include/asm-powerpc/syscall.h     |   84 +++++++++++++++++++++++++++++++++++++
>  include/asm-powerpc/thread_info.h |    5 ++-
>  9 files changed, 147 insertions(+), 36 deletions(-)
>  create mode 100644 include/asm-powerpc/syscall.h
> 
> 
> Thanks,
> Roland
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@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 0/5] powerpc tracehook

[David Miller]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Sun, 27 Jul 2008 17:52:45 +1000

> On Sat, 2008-07-26 at 23:48 -0700, Roland McGrath wrote:
> > These patches are posted for review, but you can just pull the GIT branch
> > if you prefer.  Patch 1/5 corrects a long-standing (minor) error in ptrace
> > behavior.  The others change no existing behavior, just enable new and
> > future features to work on the arch.
> 
> Hi Roland !
> 
> Thanks for this. I haven't followed the story of tracehook so far, are
> those patches dependent on something else or it's all already upstream
> and do you think that's still 2.6.27 material ?

The infrastructure is in Linus's tree.

Re: [PATCH 0/5] powerpc tracehook

[Benjamin Herrenschmidt]

> > Thanks for this. I haven't followed the story of tracehook so far, are
> > those patches dependent on something else or it's all already upstream
> > and do you think that's still 2.6.27 material ?
> 
> The infrastructure is in Linus's tree.

Thanks David. I'll review these on monday and if Paul has no objection,
I suppose we can still merge them.

Cheers,
Ben.