linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 00/27] ptrace: add PTRACE_GET_SYSCALL_INFO request
@ 2018-12-13 17:18 Dmitry V. Levin
  2018-12-13 17:24 ` [PATCH v6 26/27] " Dmitry V. Levin
  2018-12-14 20:15 ` [PATCH v6 00/27] " Paul Moore
  0 siblings, 2 replies; 3+ messages in thread
From: Dmitry V. Levin @ 2018-12-13 17:18 UTC (permalink / raw)
  To: Oleg Nesterov, Andy Lutomirski
  Cc: linux-s390, Rich Felker, linux-ia64, linux-sh,
	Benjamin Herrenschmidt, Alexey Brodkin, Heiko Carstens,
	linux-api, James E.J. Bottomley, Max Filippov, Guo Ren,
	Ralf Baechle, linux-kselftest, H. Peter Anvin, Breno Leitao,
	Russell King, linux-riscv, Vincent Chen, Shuah Khan,
	Thomas Gleixner, Paul Mackerras, Jonas Bonn, Elvira Khabirova,
	sparclinux, linux-arch

PTRACE_GET_SYSCALL_INFO is a generic ptrace API that lets ptracer obtain
details of the syscall the tracee is blocked in.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls.  Some examples include:
* The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up.  But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared.  On such architectures as ia64 this results in all syscall
arguments being unavailable for the tracer.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee.  For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

PTRACE_GET_SYSCALL_INFO returns the following structure:

struct ptrace_syscall_info {
	__u8 op;	/* PTRACE_SYSCALL_INFO_* */
	__u32 arch __attribute__((__aligned__(sizeof(__u32))));
	__u64 instruction_pointer;
	__u64 stack_pointer;
	union {
		struct {
			__u64 nr;
			__u64 args[6];
		} entry;
		struct {
			__s64 rval;
			__u8 is_error;
		} exit;
		struct {
			__u64 nr;
			__u64 args[6];
			__u32 ret_data;
		} seccomp;
	};
};

The structure was chosen according to [2], except for the following
changes:
* seccomp substructure was added as a superset of entry substructure;
* the type of nr field was changed from int to __u64 because syscall
numbers are, as a practical matter, 64 bits;
* stack_pointer field was added along with instruction_pointer field
since it is readily available and can save the tracer from extra
PTRACE_GETREGS/PTRACE_GETREGSET calls;
* arch is always initialized to aid with tracing system calls
* such as execve();
* instruction_pointer and stack_pointer are always initialized
so they could be easily obtained for non-syscall stops;
* a boolean is_error field was added along with rval field, this way
the tracer can more reliably distinguish a return value
from an error value.

strace has been ported to PTRACE_GET_SYSCALL_INFO, you can find it
in [3] and [4].

[1] https://lore.kernel.org/lkml/CA+55aFzcSVmdDj9Lh_gdbz1OzHyEm6ZrGPBDAJnywm2LF_eVyg@mail.gmail.com/
[2] https://lore.kernel.org/lkml/CAObL_7GM0n80N7J_DFw_eQyfLyzq+sf4y2AvsCCV88Tb3AwEHA@mail.gmail.com/
[3] https://github.com/strace/strace/commits/ldv/PTRACE_GET_SYSCALL_INFO
[4] https://gitlab.com/strace/strace/commits/ldv/PTRACE_GET_SYSCALL_INFO

---

Notes:
    v6:
    * Add syscall_get_arguments and syscall_set_arguments wrappers
      to asm-generic/syscall.h, requested by Geert.
    * Change PTRACE_GET_SYSCALL_INFO return code: do not take trailing paddings
      into account, use the end of the last field of the structure being written.
    * Change struct ptrace_syscall_info:
      * remove .frame_pointer field, is is not needed and not portable;
      * make .arch field explicitly aligned, remove no longer needed
        padding before .arch field;
      * remove trailing pads, they are no longer needed.

    v5:
    * Merge separate series and patches into the single series.
    * Change PTRACE_EVENTMSG_SYSCALL_{ENTRY,EXIT} values as requested by Oleg.
    * Change struct ptrace_syscall_info: generalize instruction_pointer,
      stack_pointer, and frame_pointer fields by moving them from
      ptrace_syscall_info.{entry,seccomp} substructures to ptrace_syscall_info
      and initializing them for all stops.
    * Add PTRACE_SYSCALL_INFO_NONE, set it when not in a syscall stop,
      so e.g. "strace -i" could use PTRACE_SYSCALL_INFO_SECCOMP to obtain
      instruction_pointer when the tracee is in a signal stop.
    * Patch all remaining architectures to provide all necessary
      syscall_get_* functions.
    * Make available for all architectures: do not conditionalize on
      CONFIG_HAVE_ARCH_TRACEHOOK since all syscall_get_* functions
      are implemented on all architectures.
    * Add a test for PTRACE_GET_SYSCALL_INFO to selftests/ptrace.
    
    v4:
    * Do not introduce task_struct.ptrace_event,
      use child->last_siginfo->si_code instead.
    * Implement PTRACE_SYSCALL_INFO_SECCOMP and ptrace_syscall_info.seccomp
      support along with PTRACE_SYSCALL_INFO_{ENTRY,EXIT} and
      ptrace_syscall_info.{entry,exit}.
    
    v3:
    * Change struct ptrace_syscall_info.
    * Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
    * Add proper defines for ptrace_syscall_info.op values.
    * Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
      PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT
    * and move them to uapi.
    
    v2:
    * Do not use task->ptrace.
    * Replace entry_info.is_compat with entry_info.arch, use syscall_get_arch().
    * Use addr argument of sys_ptrace to get expected size of the struct;
      return full size of the struct.

Dmitry V. Levin (25):
  asm-generic/syscall.h: prepare for inclusion by other files
  asm-generic/syscall.h: turn syscall_[gs]et_arguments into wrappers
  alpha: define remaining syscall_get_* functions
  Move EM_ARCOMPACT and EM_ARCV2 to uapi/linux/elf-em.h
  arc: define syscall_get_arch()
  c6x: define syscall_get_arch()
  elf-em.h: add EM_CSKY
  csky: define syscall_get_arch()
  h8300: define remaining syscall_get_* functions
  Move EM_HEXAGON to uapi/linux/elf-em.h
  hexagon: define remaining syscall_get_* functions
  Move EM_NDS32 to uapi/linux/elf-em.h
  nds32: define syscall_get_arch()
  nios2: define syscall_get_arch()
  m68k: add asm/syscall.h
  mips: define syscall_get_error()
  parisc: define syscall_get_error()
  powerpc: define syscall_get_error()
  riscv: define syscall_get_arch()
  Move EM_XTENSA to uapi/linux/elf-em.h
  xtensa: define syscall_get_* functions
  Move EM_UNICORE to uapi/linux/elf-em.h
  unicore32: add asm/syscall.h
  syscall_get_arch: add "struct task_struct *" argument
  selftests/ptrace: add a test case for PTRACE_GET_SYSCALL_INFO

Elvira Khabirova (2):
  powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call
  ptrace: add PTRACE_GET_SYSCALL_INFO request

 arch/alpha/include/asm/syscall.h              |  31 +-
 arch/arc/include/asm/elf.h                    |   6 +-
 arch/arc/include/asm/syscall.h                |  11 +
 arch/arm/include/asm/syscall.h                |   2 +-
 arch/arm64/include/asm/syscall.h              |   4 +-
 arch/c6x/include/asm/syscall.h                |   7 +
 arch/csky/include/asm/syscall.h               |   7 +
 arch/h8300/include/asm/syscall.h              |  19 ++
 arch/hexagon/include/asm/elf.h                |   6 +-
 arch/hexagon/include/asm/syscall.h            |  22 ++
 arch/ia64/include/asm/syscall.h               |   2 +-
 arch/m68k/include/asm/syscall.h               |  42 +++
 arch/microblaze/include/asm/syscall.h         |   2 +-
 arch/mips/include/asm/syscall.h               |  12 +-
 arch/mips/kernel/ptrace.c                     |   2 +-
 arch/nds32/include/asm/elf.h                  |   3 +-
 arch/nds32/include/asm/syscall.h              |   8 +
 arch/nios2/include/asm/syscall.h              |   6 +
 arch/openrisc/include/asm/syscall.h           |   2 +-
 arch/parisc/include/asm/syscall.h             |  11 +-
 arch/powerpc/include/asm/syscall.h            |  20 +-
 arch/powerpc/kernel/ptrace.c                  |   7 +-
 arch/riscv/include/asm/syscall.h              |  10 +
 arch/s390/include/asm/syscall.h               |   4 +-
 arch/sh/include/asm/syscall_32.h              |   2 +-
 arch/sh/include/asm/syscall_64.h              |   2 +-
 arch/sparc/include/asm/syscall.h              |   5 +-
 arch/unicore32/include/asm/elf.h              |   3 +-
 arch/unicore32/include/asm/syscall.h          |  46 +++
 arch/x86/include/asm/syscall.h                |   8 +-
 arch/x86/um/asm/syscall.h                     |   2 +-
 arch/xtensa/include/asm/elf.h                 |   2 +-
 arch/xtensa/include/asm/syscall.h             |  65 +++++
 include/asm-generic/syscall.h                 |  85 ++++--
 include/linux/tracehook.h                     |   9 +-
 include/uapi/linux/audit.h                    |  16 ++
 include/uapi/linux/elf-em.h                   |   8 +
 include/uapi/linux/ptrace.h                   |  35 +++
 kernel/auditsc.c                              |   4 +-
 kernel/ptrace.c                               | 101 ++++++-
 kernel/seccomp.c                              |   4 +-
 tools/testing/selftests/ptrace/.gitignore     |   1 +
 tools/testing/selftests/ptrace/Makefile       |   2 +-
 .../selftests/ptrace/get_syscall_info.c       | 271 ++++++++++++++++++
 44 files changed, 851 insertions(+), 66 deletions(-)
 create mode 100644 arch/m68k/include/asm/syscall.h
 create mode 100644 arch/unicore32/include/asm/syscall.h
 create mode 100644 tools/testing/selftests/ptrace/get_syscall_info.c

-- 
ldv

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v6 26/27] ptrace: add PTRACE_GET_SYSCALL_INFO request
  2018-12-13 17:18 [PATCH v6 00/27] ptrace: add PTRACE_GET_SYSCALL_INFO request Dmitry V. Levin
@ 2018-12-13 17:24 ` Dmitry V. Levin
  2018-12-14 20:15 ` [PATCH v6 00/27] " Paul Moore
  1 sibling, 0 replies; 3+ messages in thread
From: Dmitry V. Levin @ 2018-12-13 17:24 UTC (permalink / raw)
  To: Andy Lutomirski, Oleg Nesterov
  Cc: Eugene Syromyatnikov, Kees Cook, Jann Horn, linux-api,
	strace-devel, linux-kernel

From: Elvira Khabirova <lineprinter@altlinux.org>

PTRACE_GET_SYSCALL_INFO is a generic ptrace API that lets ptracer obtain
details of the syscall the tracee is blocked in.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls.  Some examples include:
* The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up.  But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared.  On such architectures as ia64 this results in all syscall
arguments being unavailable for the tracer.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee.  For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

ptrace(2) man page:

long ptrace(enum __ptrace_request request, pid_t pid,
            void *addr, void *data);
...
PTRACE_GET_SYSCALL_INFO
       Retrieve information about the syscall that caused the stop.
       The information is placed into the buffer pointed by "data"
       argument, which should be a pointer to a buffer of type
       "struct ptrace_syscall_info".
       The "addr" argument contains the size of the buffer pointed to
       by "data" argument (i.e., sizeof(struct ptrace_syscall_info)).
       The return value contains the number of bytes available
       to be written by the kernel.
       If the size of data to be written by the kernel exceeds the size
       specified by "addr" argument, the output is truncated.

Co-authored-by: Dmitry V. Levin <ldv@altlinux.org>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Eugene Syromyatnikov <esyr@redhat.com>
Cc: Jann Horn <jannh@google.com>
Cc: linux-api@vger.kernel.org
Cc: strace-devel@lists.strace.io
Signed-off-by: Elvira Khabirova <lineprinter@altlinux.org>
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---

Notes:
    v6:
    * Change PTRACE_GET_SYSCALL_INFO return code: do not take trailing paddings
      into account, use the end of the last field of the structure being written.
    * Change struct ptrace_syscall_info:
      * remove .frame_pointer field, is is not needed and not portable;
      * make .arch field explicitly aligned, remove no longer needed
        padding before .arch field;
    * remove trailing pads, they are no longer needed.
    
    v5:
    * Change PTRACE_EVENTMSG_SYSCALL_{ENTRY,EXIT} values as requested by Oleg.
    * Change struct ptrace_syscall_info: generalize instruction_pointer,
      stack_pointer, and frame_pointer fields by moving them from
      ptrace_syscall_info.{entry,seccomp} substructures to ptrace_syscall_info
      and initializing them for all stops.
    * Add PTRACE_SYSCALL_INFO_NONE, set it when not in a syscall stop,
      so e.g. "strace -i" could use PTRACE_SYSCALL_INFO_SECCOMP to obtain
      instruction_pointer when the tracee is in a signal stop.
    * Make available for all architectures: do not conditionalize on
      CONFIG_HAVE_ARCH_TRACEHOOK since all syscall_get_* functions
      are implemented on all architectures.
    
    v4:
    * Do not introduce task_struct.ptrace_event,
      use child->last_siginfo->si_code instead.
    * Implement PTRACE_SYSCALL_INFO_SECCOMP and ptrace_syscall_info.seccomp
      support along with PTRACE_SYSCALL_INFO_{ENTRY,EXIT} and
      ptrace_syscall_info.{entry,exit}.
    
    v3:
    * Change struct ptrace_syscall_info.
    * Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
    * Add proper defines for ptrace_syscall_info.op values.
    * Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
      PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT
    * and move them to uapi.
    
    v2:
    * Do not use task->ptrace.
    * Replace entry_info.is_compat with entry_info.arch, use syscall_get_arch().
    * Use addr argument of sys_ptrace to get expected size of the struct;
      return full size of the struct.

 include/linux/tracehook.h   |   9 ++--
 include/uapi/linux/ptrace.h |  35 +++++++++++++
 kernel/ptrace.c             | 101 +++++++++++++++++++++++++++++++++++-
 3 files changed, 141 insertions(+), 4 deletions(-)

diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index df20f8bdbfa3..6bc7a3d58e2f 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -57,13 +57,15 @@ struct linux_binprm;
 /*
  * ptrace report for syscall entry and exit looks identical.
  */
-static inline int ptrace_report_syscall(struct pt_regs *regs)
+static inline int ptrace_report_syscall(struct pt_regs *regs,
+					unsigned long message)
 {
 	int ptrace = current->ptrace;
 
 	if (!(ptrace & PT_PTRACED))
 		return 0;
 
+	current->ptrace_message = message;
 	ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
 
 	/*
@@ -76,6 +78,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
 		current->exit_code = 0;
 	}
 
+	current->ptrace_message = 0;
 	return fatal_signal_pending(current);
 }
 
@@ -101,7 +104,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
 static inline __must_check int tracehook_report_syscall_entry(
 	struct pt_regs *regs)
 {
-	return ptrace_report_syscall(regs);
+	return ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_ENTRY);
 }
 
 /**
@@ -126,7 +129,7 @@ static inline void tracehook_report_syscall_exit(struct pt_regs *regs, int step)
 	if (step)
 		user_single_step_report(regs);
 	else
-		ptrace_report_syscall(regs);
+		ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_EXIT);
 }
 
 /**
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index d5a1b8a492b9..a71b6e3b03eb 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -73,6 +73,41 @@ struct seccomp_metadata {
 	__u64 flags;		/* Output: filter's flags */
 };
 
+#define PTRACE_GET_SYSCALL_INFO		0x420e
+#define PTRACE_SYSCALL_INFO_NONE	0
+#define PTRACE_SYSCALL_INFO_ENTRY	1
+#define PTRACE_SYSCALL_INFO_EXIT	2
+#define PTRACE_SYSCALL_INFO_SECCOMP	3
+
+struct ptrace_syscall_info {
+	__u8 op;	/* PTRACE_SYSCALL_INFO_* */
+	__u32 arch __attribute__((__aligned__(sizeof(__u32))));
+	__u64 instruction_pointer;
+	__u64 stack_pointer;
+	union {
+		struct {
+			__u64 nr;
+			__u64 args[6];
+		} entry;
+		struct {
+			__s64 rval;
+			__u8 is_error;
+		} exit;
+		struct {
+			__u64 nr;
+			__u64 args[6];
+			__u32 ret_data;
+		} seccomp;
+	};
+};
+
+/*
+ * These values are stored in task->ptrace_message
+ * by tracehook_report_syscall_* to describe the current syscall-stop.
+ */
+#define PTRACE_EVENTMSG_SYSCALL_ENTRY	1
+#define PTRACE_EVENTMSG_SYSCALL_EXIT	2
+
 /* Read signals from a shared (process wide) queue */
 #define PTRACE_PEEKSIGINFO_SHARED	(1 << 0)
 
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index c2cee9db5204..2e5faeeaf4cd 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -30,6 +30,8 @@
 #include <linux/cn_proc.h>
 #include <linux/compat.h>
 
+#include <asm/syscall.h>	/* For syscall_get_* */
+
 /*
  * Access another process' address space via ptrace.
  * Source/target buffer must be kernel space,
@@ -878,7 +880,100 @@ static int ptrace_regset(struct task_struct *task, int req, unsigned int type,
  * to ensure no machine forgets it.
  */
 EXPORT_SYMBOL_GPL(task_user_regset_view);
-#endif
+#endif /* CONFIG_HAVE_ARCH_TRACEHOOK */
+
+static unsigned long
+ptrace_get_syscall_info_entry(struct task_struct *child, struct pt_regs *regs,
+			      struct ptrace_syscall_info *info)
+{
+	unsigned long args[ARRAY_SIZE(info->entry.args)];
+	int i;
+
+	info->op = PTRACE_SYSCALL_INFO_ENTRY;
+	info->entry.nr = syscall_get_nr(child, regs);
+	syscall_get_arguments(child, regs, 0, ARRAY_SIZE(args), args);
+	for (i = 0; i < ARRAY_SIZE(args); i++)
+		info->entry.args[i] = args[i];
+
+	/* args is the last field in struct ptrace_syscall_info.entry */
+	return offsetofend(struct ptrace_syscall_info, entry.args);
+}
+
+static unsigned long
+ptrace_get_syscall_info_seccomp(struct task_struct *child, struct pt_regs *regs,
+				struct ptrace_syscall_info *info)
+{
+	/*
+	 * As struct ptrace_syscall_info.entry is currently a subset
+	 * of struct ptrace_syscall_info.seccomp, it makes sense to
+	 * initialize that subset using ptrace_get_syscall_info_entry().
+	 * This can be reconsidered in the future if these structures
+	 * diverge significantly enough.
+	 */
+	ptrace_get_syscall_info_entry(child, regs, info);
+	info->op = PTRACE_SYSCALL_INFO_SECCOMP;
+	info->seccomp.ret_data = child->ptrace_message;
+
+	/* ret_data is the last field in struct ptrace_syscall_info.seccomp */
+	return offsetofend(struct ptrace_syscall_info, seccomp.ret_data);
+}
+
+static unsigned long
+ptrace_get_syscall_info_exit(struct task_struct *child, struct pt_regs *regs,
+			     struct ptrace_syscall_info *info)
+{
+	info->op = PTRACE_SYSCALL_INFO_EXIT;
+	info->exit.rval = syscall_get_error(child, regs);
+	info->exit.is_error = !!info->exit.rval;
+	if (!info->exit.is_error)
+		info->exit.rval = syscall_get_return_value(child, regs);
+
+	/* is_error is the last field in struct ptrace_syscall_info.exit */
+	return offsetofend(struct ptrace_syscall_info, exit.is_error);
+}
+
+static int
+ptrace_get_syscall_info(struct task_struct *child, unsigned long user_size,
+			void __user *datavp)
+{
+	struct pt_regs *regs = task_pt_regs(child);
+	struct ptrace_syscall_info info = {
+		.op = PTRACE_SYSCALL_INFO_NONE,
+		.arch = syscall_get_arch(child),
+		.instruction_pointer = instruction_pointer(regs),
+		.stack_pointer = user_stack_pointer(regs),
+	};
+	unsigned long actual_size = offsetof(struct ptrace_syscall_info, entry);
+	unsigned long write_size;
+
+	/*
+	 * This does not need lock_task_sighand() to access
+	 * child->last_siginfo because ptrace_freeze_traced()
+	 * called earlier by ptrace_check_attach() ensures that
+	 * the tracee cannot go away and clear its last_siginfo.
+	 */
+	switch (child->last_siginfo ? child->last_siginfo->si_code : 0) {
+	case SIGTRAP | 0x80:
+		switch (child->ptrace_message) {
+		case PTRACE_EVENTMSG_SYSCALL_ENTRY:
+			actual_size = ptrace_get_syscall_info_entry(child, regs,
+								    &info);
+			break;
+		case PTRACE_EVENTMSG_SYSCALL_EXIT:
+			actual_size = ptrace_get_syscall_info_exit(child, regs,
+								   &info);
+			break;
+		}
+		break;
+	case SIGTRAP | (PTRACE_EVENT_SECCOMP << 8):
+		actual_size = ptrace_get_syscall_info_seccomp(child, regs,
+							      &info);
+		break;
+	}
+
+	write_size = min(actual_size, user_size);
+	return copy_to_user(datavp, &info, write_size) ? -EFAULT : actual_size;
+}
 
 int ptrace_request(struct task_struct *child, long request,
 		   unsigned long addr, unsigned long data)
@@ -1095,6 +1190,10 @@ int ptrace_request(struct task_struct *child, long request,
 		ret = seccomp_get_metadata(child, addr, datavp);
 		break;
 
+	case PTRACE_GET_SYSCALL_INFO:
+		ret = ptrace_get_syscall_info(child, addr, datavp);
+		break;
+
 	default:
 		break;
 	}
-- 
ldv

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v6 00/27] ptrace: add PTRACE_GET_SYSCALL_INFO request
  2018-12-13 17:18 [PATCH v6 00/27] ptrace: add PTRACE_GET_SYSCALL_INFO request Dmitry V. Levin
  2018-12-13 17:24 ` [PATCH v6 26/27] " Dmitry V. Levin
@ 2018-12-14 20:15 ` Paul Moore
  1 sibling, 0 replies; 3+ messages in thread
From: Paul Moore @ 2018-12-14 20:15 UTC (permalink / raw)
  To: ldv
  Cc: linux-snps-arc, dalias, linux-ia64, linux-sh, benh,
	alexey.brodkin, heiko.carstens, linux-api, jejb, jcmvbkbc,
	guoren, ralf, linux-kselftest, hpa, leitao, linux, linux-riscv,
	deanbo422, shuah, tglx, paulus, jonas, linux-s390, sparclinux,
	linux-arch, linux-c6x-dev, ysato, linux-xtensa, mpe, deller, x86,
	linux-kernel, esyr, linux-alpha, lineprinter, geert,
	catalin.marinas

On Thu, Dec 13, 2018 at 12:18 PM Dmitry V. Levin <ldv@altlinux.org> wrote:
> PTRACE_GET_SYSCALL_INFO is a generic ptrace API that lets ptracer obtain
> details of the syscall the tracee is blocked in.
>
> There are two reasons for a special syscall-related ptrace request.
>
> Firstly, with the current ptrace API there are cases when ptracer cannot
> retrieve necessary information about syscalls.  Some examples include:
> * The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
> In short, if a 64-bit task performs a syscall through int 0x80, its tracer
> has no reliable means to find out that the syscall was, in fact,
> a compat syscall, and misidentifies it.
> * Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
> Common practice is to keep track of the sequence of ptrace-stops in order
> not to mix the two syscall-stops up.  But it is not as simple as it looks;
> for example, strace had a (just recently fixed) long-standing bug where
> attaching strace to a tracee that is performing the execve system call
> led to the tracer identifying the following syscall-exit-stop as
> syscall-enter-stop, which messed up all the state tracking.
> * Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
> ("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
> and process_vm_readv become unavailable when the process dumpable flag
> is cleared.  On such architectures as ia64 this results in all syscall
> arguments being unavailable for the tracer.
>
> Secondly, ptracers also have to support a lot of arch-specific code for
> obtaining information about the tracee.  For some architectures, this
> requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
> argument and return value.
>
> PTRACE_GET_SYSCALL_INFO returns the following structure:
>
> struct ptrace_syscall_info {
>         __u8 op;        /* PTRACE_SYSCALL_INFO_* */
>         __u32 arch __attribute__((__aligned__(sizeof(__u32))));
>         __u64 instruction_pointer;
>         __u64 stack_pointer;
>         union {
>                 struct {
>                         __u64 nr;
>                         __u64 args[6];
>                 } entry;
>                 struct {
>                         __s64 rval;
>                         __u8 is_error;
>                 } exit;
>                 struct {
>                         __u64 nr;
>                         __u64 args[6];
>                         __u32 ret_data;
>                 } seccomp;
>         };
> };
>
> The structure was chosen according to [2], except for the following
> changes:
> * seccomp substructure was added as a superset of entry substructure;
> * the type of nr field was changed from int to __u64 because syscall
> numbers are, as a practical matter, 64 bits;
> * stack_pointer field was added along with instruction_pointer field
> since it is readily available and can save the tracer from extra
> PTRACE_GETREGS/PTRACE_GETREGSET calls;
> * arch is always initialized to aid with tracing system calls
> * such as execve();
> * instruction_pointer and stack_pointer are always initialized
> so they could be easily obtained for non-syscall stops;
> * a boolean is_error field was added along with rval field, this way
> the tracer can more reliably distinguish a return value
> from an error value.
>
> strace has been ported to PTRACE_GET_SYSCALL_INFO, you can find it
> in [3] and [4].
>
> [1] https://lore.kernel.org/lkml/CA+55aFzcSVmdDj9Lh_gdbz1OzHyEm6ZrGPBDAJnywm2LF_eVyg@mail.gmail.com/
> [2] https://lore.kernel.org/lkml/CAObL_7GM0n80N7J_DFw_eQyfLyzq+sf4y2AvsCCV88Tb3AwEHA@mail.gmail.com/
> [3] https://github.com/strace/strace/commits/ldv/PTRACE_GET_SYSCALL_INFO
> [4] https://gitlab.com/strace/strace/commits/ldv/PTRACE_GET_SYSCALL_INFO
>
> ---
>
> Notes:
>     v6:
>     * Add syscall_get_arguments and syscall_set_arguments wrappers
>       to asm-generic/syscall.h, requested by Geert.
>     * Change PTRACE_GET_SYSCALL_INFO return code: do not take trailing paddings
>       into account, use the end of the last field of the structure being written.
>     * Change struct ptrace_syscall_info:
>       * remove .frame_pointer field, is is not needed and not portable;
>       * make .arch field explicitly aligned, remove no longer needed
>         padding before .arch field;
>       * remove trailing pads, they are no longer needed.
>
>     v5:
>     * Merge separate series and patches into the single series.
>     * Change PTRACE_EVENTMSG_SYSCALL_{ENTRY,EXIT} values as requested by Oleg.
>     * Change struct ptrace_syscall_info: generalize instruction_pointer,
>       stack_pointer, and frame_pointer fields by moving them from
>       ptrace_syscall_info.{entry,seccomp} substructures to ptrace_syscall_info
>       and initializing them for all stops.
>     * Add PTRACE_SYSCALL_INFO_NONE, set it when not in a syscall stop,
>       so e.g. "strace -i" could use PTRACE_SYSCALL_INFO_SECCOMP to obtain
>       instruction_pointer when the tracee is in a signal stop.
>     * Patch all remaining architectures to provide all necessary
>       syscall_get_* functions.
>     * Make available for all architectures: do not conditionalize on
>       CONFIG_HAVE_ARCH_TRACEHOOK since all syscall_get_* functions
>       are implemented on all architectures.
>     * Add a test for PTRACE_GET_SYSCALL_INFO to selftests/ptrace.
>
>     v4:
>     * Do not introduce task_struct.ptrace_event,
>       use child->last_siginfo->si_code instead.
>     * Implement PTRACE_SYSCALL_INFO_SECCOMP and ptrace_syscall_info.seccomp
>       support along with PTRACE_SYSCALL_INFO_{ENTRY,EXIT} and
>       ptrace_syscall_info.{entry,exit}.
>
>     v3:
>     * Change struct ptrace_syscall_info.
>     * Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
>     * Add proper defines for ptrace_syscall_info.op values.
>     * Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
>       PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT
>     * and move them to uapi.
>
>     v2:
>     * Do not use task->ptrace.
>     * Replace entry_info.is_compat with entry_info.arch, use syscall_get_arch().
>     * Use addr argument of sys_ptrace to get expected size of the struct;
>       return full size of the struct.
>
> Dmitry V. Levin (25):
>   asm-generic/syscall.h: prepare for inclusion by other files
>   asm-generic/syscall.h: turn syscall_[gs]et_arguments into wrappers
>   alpha: define remaining syscall_get_* functions
>   Move EM_ARCOMPACT and EM_ARCV2 to uapi/linux/elf-em.h
>   arc: define syscall_get_arch()
>   c6x: define syscall_get_arch()
>   elf-em.h: add EM_CSKY
>   csky: define syscall_get_arch()
>   h8300: define remaining syscall_get_* functions
>   Move EM_HEXAGON to uapi/linux/elf-em.h
>   hexagon: define remaining syscall_get_* functions
>   Move EM_NDS32 to uapi/linux/elf-em.h
>   nds32: define syscall_get_arch()
>   nios2: define syscall_get_arch()
>   m68k: add asm/syscall.h
>   mips: define syscall_get_error()
>   parisc: define syscall_get_error()
>   powerpc: define syscall_get_error()
>   riscv: define syscall_get_arch()
>   Move EM_XTENSA to uapi/linux/elf-em.h
>   xtensa: define syscall_get_* functions
>   Move EM_UNICORE to uapi/linux/elf-em.h
>   unicore32: add asm/syscall.h
>   syscall_get_arch: add "struct task_struct *" argument
>   selftests/ptrace: add a test case for PTRACE_GET_SYSCALL_INFO
>
> Elvira Khabirova (2):
>   powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call
>   ptrace: add PTRACE_GET_SYSCALL_INFO request

As mentioned in the previous patchsets, this all looks fine to me from
an audit perspective (although there is not much audit code to really
comment on).  Feel free to add my ACK to the audit related patches.

Acked-by: Paul Moore <paul@paul-moore.com>

>  arch/alpha/include/asm/syscall.h              |  31 +-
>  arch/arc/include/asm/elf.h                    |   6 +-
>  arch/arc/include/asm/syscall.h                |  11 +
>  arch/arm/include/asm/syscall.h                |   2 +-
>  arch/arm64/include/asm/syscall.h              |   4 +-
>  arch/c6x/include/asm/syscall.h                |   7 +
>  arch/csky/include/asm/syscall.h               |   7 +
>  arch/h8300/include/asm/syscall.h              |  19 ++
>  arch/hexagon/include/asm/elf.h                |   6 +-
>  arch/hexagon/include/asm/syscall.h            |  22 ++
>  arch/ia64/include/asm/syscall.h               |   2 +-
>  arch/m68k/include/asm/syscall.h               |  42 +++
>  arch/microblaze/include/asm/syscall.h         |   2 +-
>  arch/mips/include/asm/syscall.h               |  12 +-
>  arch/mips/kernel/ptrace.c                     |   2 +-
>  arch/nds32/include/asm/elf.h                  |   3 +-
>  arch/nds32/include/asm/syscall.h              |   8 +
>  arch/nios2/include/asm/syscall.h              |   6 +
>  arch/openrisc/include/asm/syscall.h           |   2 +-
>  arch/parisc/include/asm/syscall.h             |  11 +-
>  arch/powerpc/include/asm/syscall.h            |  20 +-
>  arch/powerpc/kernel/ptrace.c                  |   7 +-
>  arch/riscv/include/asm/syscall.h              |  10 +
>  arch/s390/include/asm/syscall.h               |   4 +-
>  arch/sh/include/asm/syscall_32.h              |   2 +-
>  arch/sh/include/asm/syscall_64.h              |   2 +-
>  arch/sparc/include/asm/syscall.h              |   5 +-
>  arch/unicore32/include/asm/elf.h              |   3 +-
>  arch/unicore32/include/asm/syscall.h          |  46 +++
>  arch/x86/include/asm/syscall.h                |   8 +-
>  arch/x86/um/asm/syscall.h                     |   2 +-
>  arch/xtensa/include/asm/elf.h                 |   2 +-
>  arch/xtensa/include/asm/syscall.h             |  65 +++++
>  include/asm-generic/syscall.h                 |  85 ++++--
>  include/linux/tracehook.h                     |   9 +-
>  include/uapi/linux/audit.h                    |  16 ++
>  include/uapi/linux/elf-em.h                   |   8 +
>  include/uapi/linux/ptrace.h                   |  35 +++
>  kernel/auditsc.c                              |   4 +-
>  kernel/ptrace.c                               | 101 ++++++-
>  kernel/seccomp.c                              |   4 +-
>  tools/testing/selftests/ptrace/.gitignore     |   1 +
>  tools/testing/selftests/ptrace/Makefile       |   2 +-
>  .../selftests/ptrace/get_syscall_info.c       | 271 ++++++++++++++++++
>  44 files changed, 851 insertions(+), 66 deletions(-)
>  create mode 100644 arch/m68k/include/asm/syscall.h
>  create mode 100644 arch/unicore32/include/asm/syscall.h
>  create mode 100644 tools/testing/selftests/ptrace/get_syscall_info.c
>
> --
> ldv



--
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-12-14 20:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-13 17:18 [PATCH v6 00/27] ptrace: add PTRACE_GET_SYSCALL_INFO request Dmitry V. Levin
2018-12-13 17:24 ` [PATCH v6 26/27] " Dmitry V. Levin
2018-12-14 20:15 ` [PATCH v6 00/27] " Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).