[PATCH v2 0/2] ARM: ptrace: Fix seccomp seeing stale syscall arguments

[PATCH v2 0/2] ARM: ptrace: Fix seccomp seeing stale syscall arguments

[Jinjie Ruan]

On arm32 the syscall entry code copies r0 into ARM_ORIG_r0, and seccomp,
the sys_enter tracepoint and the syscall restart path read the first
argument from that copy, while audit and the syscall itself runs
with r0.

If a tracer rewrites r0 before the syscall is dispatched, at a
syscall-entry stop or at a SECCOMP_RET_TRACE stop, leaves ARM_ORIG_r0
stale: seccomp and the tracepoint see the old argument while audit and
the syscall runs with the new one.

arm64 has the same problem with orig_x0 and it is already fixed there in
commit 88b839ce497c ("arm64: ptrace: Keep 'orig_x0' in-sync with x0 on
syscall entry").

The fisrt patch fix this issue, and the second add testcases.

Changes since v1:
- Fix the sashiko issues.
- As Linus suggested, expand the explanation content from
  the previous version into the commit message.
- Add the selftests.
- Link to v1: https://lore.kernel.org/all/20260725091452.1875627-1-ruanjinjie@huawei.com/

- Link: https://sashiko.dev/#/patchset/20260725091452.1875627-1-ruanjinjie%40huawei.com

Jinjie Ruan (2):
  ARM: ptrace: Fix seccomp seeing stale syscall arguments
  selftests/seccomp: Add tests for syscall argument rewrite

 arch/arm/kernel/ptrace.c                      |  41 +-
 tools/testing/selftests/seccomp/.gitignore    |   1 +
 tools/testing/selftests/seccomp/Makefile      |   2 +-
 .../selftests/seccomp/seccomp_ptrace_args.c   | 463 ++++++++++++++++++
 4 files changed, 505 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/seccomp/seccomp_ptrace_args.c

-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 0/2] ARM: ptrace: Fix seccomp seeing stale syscall arguments

[Jinjie Ruan]

On arm32 the syscall entry code copies r0 into ARM_ORIG_r0, and seccomp,
the sys_enter tracepoint and the syscall restart path read the first
argument from that copy, while audit and the syscall itself runs
with r0.

If a tracer rewrites r0 before the syscall is dispatched, at a
syscall-entry stop or at a SECCOMP_RET_TRACE stop, leaves ARM_ORIG_r0
stale: seccomp and the tracepoint see the old argument while audit and
the syscall runs with the new one.

arm64 has the same problem with orig_x0 and it is already fixed there in
commit 88b839ce497c ("arm64: ptrace: Keep 'orig_x0' in-sync with x0 on
syscall entry").

The fisrt patch fix this issue, and the second add testcases.

Changes since v1:
- Fix the sashiko issues.
- As Linus suggested, expand the explanation content from
  the previous version into the commit message.
- Add the selftests.
- Link to v1: https://lore.kernel.org/all/20260725091452.1875627-1-ruanjinjie@huawei.com/

- Link: https://sashiko.dev/#/patchset/20260725091452.1875627-1-ruanjinjie%40huawei.com

Jinjie Ruan (2):
  ARM: ptrace: Fix seccomp seeing stale syscall arguments
  selftests/seccomp: Add tests for syscall argument rewrite

 arch/arm/kernel/ptrace.c                      |  41 +-
 tools/testing/selftests/seccomp/.gitignore    |   1 +
 tools/testing/selftests/seccomp/Makefile      |   2 +-
 .../selftests/seccomp/seccomp_ptrace_args.c   | 463 ++++++++++++++++++
 4 files changed, 505 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/seccomp/seccomp_ptrace_args.c

-- 
2.34.1

[PATCH v2 1/2] ARM: ptrace: Fix seccomp seeing stale syscall arguments

[Jinjie Ruan]

[Background]
When user space enters a syscall, the ARM entry code copies r0 into
ARM_ORIG_r0.  r0 carries the first syscall argument, but the syscall
also returns its value in r0, so the kernel keeps a copy.

4 places use the first argument:
 - seccomp and the sys_enter tracepoint read ARM_ORIG_r0, through
   syscall_get_arguments();
 - audit_syscall_entry() reads r0;
 - the syscall itself runs with r0;
 - do_signal() copies ARM_ORIG_r0 back into r0 when a syscall restarts.

[The problem]
A tracer can change r0 while the tracee is stopped before the syscall
runs, at a syscall-entry stop (PTRACE_SETREGS, PTRACE_POKEUSR) or at a
SECCOMP_RET_TRACE stop.  Only r0 changes then, so ARM_ORIG_r0 keeps the
old value:
 - seccomp and the tracepoint see the old argument, while the syscall
   runs with the new one.  With SECCOMP_RET_TRACE this is easy to hit:
   the tracer changes the argument, the filters run again, and they
   still match on the old value;
 - a syscall that returns -ERESTARTSYS restarts with the old argument,
   because the restart path copies ARM_ORIG_r0 into r0.

A tracer that uses SECCOMP_RET_TRACE to check syscalls changes the
first argument at the PTRACE_EVENT_SECCOMP stop, and expects
the filters that run again to see the new value.

arm64 has the same problem and fixed in commit 88b839ce497c ("arm64:
ptrace: Keep 'orig_x0' in-sync with x0 on syscall entry").

x86 does not have this problem.  32-bit x86 saves only the syscall
number in orig_ax, and every reader gets it back with syscall_get_nr():

  do_int80_emulation()
    -> regs->orig_ax = regs->ax & GENMASK(31, 0)
       regs->ax = -ENOSYS
    -> nr = syscall_32_enter()
       -> regs->orig_ax
    -> nr = syscall_enter_from_user_mode_work(regs, &nr)
         -> syscall_trace_enter()
              -> arch_ptrace_report_syscall_permit_entry()
              -> __seccomp_permit_syscall()
                 -> syscall_get_nr() -> regs->orig_ax
              -> trace_syscall_enter()
                 -> syscall_get_nr() -> regs->orig_ax
              -> syscall_enter_audit()
                  -> syscall_get_nr() -> regs->orig_ax
         -> nr = syscall_get_nr() -> regs->orig_ax
    -> do_syscall_32_irqs_on(regs, nr) // Use orig_ax as the system call number
                                       // to execute the system call. This is\
                                       // consistent with seccomp, audit, and
                                       // tracepoint.

arm32 instead has two copies of the first argument.  seccomp and the
tracepoints read one copy, and the syscall runs with the other:

  vector_swi
    -> str r0, [sp, #S_OLD_R0]          // backup r0 to ARM_ORIG_r0
    -> syscall_trace_enter()
         -> report_syscall()
         -> seccomp_permit_syscall()
              -> syscall_get_arguments() -> ARM_ORIG_r0
         -> trace_sys_enter()
              -> syscall_get_arguments() -> ARM_ORIG_r0
         -> audit_syscall_entry(scno, ARM_r0, ARM_r1, ARM_r2, ARM_r3)
				      ^^^^^^
    -> invoke_syscall, reload r0-r6
       -> use r0 here!
             ^^^^

So after a tracer changes r0, seccomp and the tracepoints still read the
old ARM_ORIG_r0, audit reads the new ARM_r0, and the syscall runs with
the new ARM_r0.

riscv and loongarch also keep a copy of the first argument (orig_a0),
but they pass that copy to both the syscall and
syscall_get_arguments(), so they stay consistent.  On arm32 the syscall
runs with a different register than the one seccomp and the tracepoints
read.

The syscall number has a similar copy (abi_syscall), but there the
readers and the dispatcher both use syscall_get_nr(), so only the first
argument is affected.

[Fix]
Copy r0 into ARM_ORIG_r0 after a ptrace register write, but only when
the tracee is stopped before its syscall runs:
 - at a syscall-entry stop or a SECCOMP_RET_TRACE stop, r0 still holds
   the first argument, and seccomp, the tracepoints, audit and the
   syscall itself are about to read it;
 - at a syscall-exit stop, r0 holds the return value (-ERESTARTSYS and
   friends).  ARM_ORIG_r0 must stay, or a restarted syscall would run
   with the error code as its first argument;
 - for any other exception, the entry code sets ARM_ORIG_r0 to ~0 and it
   has no meaning, so it must not be overwritten.

The stop type is the same classification that
ptrace_get_syscall_info_op() uses for PTRACE_GET_SYSCALL_INFO.

audit_syscall_entry() takes the first argument from r0 directly.  Give
it ARM_ORIG_r0, as seccomp and the tracepoints do, so audit describes
the syscall that really runs.

Tested on arm32 with a real gdb in qemu-system-arm: after
"catch syscall getpid" the tracer sets r0 to a value the seccomp filter
rejects.  With this patch the filter refuses the syscall, without it the
syscall runs with the old argument.

Cc: stable@vger.kernel.org
Fixes: bf2c9f986692 ("ARM: 7373/1: add support for the generic syscall.h interface")
Link: https://lore.kernel.org/all/al98jdcleu-6tMDL@willie-the-truck/
Suggested-by: Will Deacon <will@kernel.org>
Link: https://lkml.org/lkml/2020/9/11/1282
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 arch/arm/kernel/ptrace.c | 41 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index ed7a2a87a670..6ec81e5a205e 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -163,6 +163,42 @@ static inline long get_user_reg(struct task_struct *task, int offset)
 	return task_pt_regs(task)->uregs[offset];
 }
 
+/*
+ * Is @target stopped after entering a syscall, but before it has been
+ * dispatched?  This is the same classification that
+ * ptrace_get_syscall_info_op() performs for PTRACE_GET_SYSCALL_INFO.
+ */
+static bool stopped_in_syscall_entry(struct task_struct *target)
+{
+	struct kernel_siginfo *info = target->last_siginfo;
+
+	if (!info)
+		return false;
+
+	if (info->si_code == (SIGTRAP | 0x80))
+		return target->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY;
+
+	return info->si_code == (SIGTRAP | (PTRACE_EVENT_SECCOMP << 8));
+}
+
+static void update_syscall_orig_r0_after_ptrace(struct task_struct *target)
+{
+	struct pt_regs *regs = task_pt_regs(target);
+
+	/*
+	 * ARM_r0 holds the first syscall argument only before the syscall is
+	 * dispatched; seccomp, the syscall tracepoints and the syscall restart
+	 * code all read that argument from ARM_ORIG_r0 instead.  At a syscall
+	 * exit stop ARM_r0 holds the return value and must not be copied, and
+	 * for other exceptions ARM_ORIG_r0 is set to ~0 and carries no
+	 * meaning.
+	 */
+	if (!stopped_in_syscall_entry(target))
+		return;
+
+	regs->ARM_ORIG_r0 = regs->ARM_r0;
+}
+
 /*
  * this routine will put a word on the processes privileged stack.
  * the offset is how far from the base addr as stored in the THREAD.
@@ -180,6 +216,8 @@ put_user_reg(struct task_struct *task, int offset, long data)
 
 	if (valid_user_regs(&newregs)) {
 		regs->uregs[offset] = data;
+		if (offset == 0)
+			update_syscall_orig_r0_after_ptrace(task);
 		ret = 0;
 	}
 
@@ -566,6 +604,7 @@ static int gpr_set(struct task_struct *target,
 		return -EINVAL;
 
 	*task_pt_regs(target) = newregs;
+	update_syscall_orig_r0_after_ptrace(target);
 	return 0;
 }
 
@@ -868,7 +907,7 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs)
 	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
 		trace_sys_enter(regs, scno);
 
-	audit_syscall_entry(scno, regs->ARM_r0, regs->ARM_r1, regs->ARM_r2,
+	audit_syscall_entry(scno, regs->ARM_ORIG_r0, regs->ARM_r1, regs->ARM_r2,
 			    regs->ARM_r3);
 
 	return scno;
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 1/2] ARM: ptrace: Fix seccomp seeing stale syscall arguments

[Jinjie Ruan]

[Background]
When user space enters a syscall, the ARM entry code copies r0 into
ARM_ORIG_r0.  r0 carries the first syscall argument, but the syscall
also returns its value in r0, so the kernel keeps a copy.

4 places use the first argument:
 - seccomp and the sys_enter tracepoint read ARM_ORIG_r0, through
   syscall_get_arguments();
 - audit_syscall_entry() reads r0;
 - the syscall itself runs with r0;
 - do_signal() copies ARM_ORIG_r0 back into r0 when a syscall restarts.

[The problem]
A tracer can change r0 while the tracee is stopped before the syscall
runs, at a syscall-entry stop (PTRACE_SETREGS, PTRACE_POKEUSR) or at a
SECCOMP_RET_TRACE stop.  Only r0 changes then, so ARM_ORIG_r0 keeps the
old value:
 - seccomp and the tracepoint see the old argument, while the syscall
   runs with the new one.  With SECCOMP_RET_TRACE this is easy to hit:
   the tracer changes the argument, the filters run again, and they
   still match on the old value;
 - a syscall that returns -ERESTARTSYS restarts with the old argument,
   because the restart path copies ARM_ORIG_r0 into r0.

A tracer that uses SECCOMP_RET_TRACE to check syscalls changes the
first argument at the PTRACE_EVENT_SECCOMP stop, and expects
the filters that run again to see the new value.

arm64 has the same problem and fixed in commit 88b839ce497c ("arm64:
ptrace: Keep 'orig_x0' in-sync with x0 on syscall entry").

x86 does not have this problem.  32-bit x86 saves only the syscall
number in orig_ax, and every reader gets it back with syscall_get_nr():

  do_int80_emulation()
    -> regs->orig_ax = regs->ax & GENMASK(31, 0)
       regs->ax = -ENOSYS
    -> nr = syscall_32_enter()
       -> regs->orig_ax
    -> nr = syscall_enter_from_user_mode_work(regs, &nr)
         -> syscall_trace_enter()
              -> arch_ptrace_report_syscall_permit_entry()
              -> __seccomp_permit_syscall()
                 -> syscall_get_nr() -> regs->orig_ax
              -> trace_syscall_enter()
                 -> syscall_get_nr() -> regs->orig_ax
              -> syscall_enter_audit()
                  -> syscall_get_nr() -> regs->orig_ax
         -> nr = syscall_get_nr() -> regs->orig_ax
    -> do_syscall_32_irqs_on(regs, nr) // Use orig_ax as the system call number
                                       // to execute the system call. This is\
                                       // consistent with seccomp, audit, and
                                       // tracepoint.

arm32 instead has two copies of the first argument.  seccomp and the
tracepoints read one copy, and the syscall runs with the other:

  vector_swi
    -> str r0, [sp, #S_OLD_R0]          // backup r0 to ARM_ORIG_r0
    -> syscall_trace_enter()
         -> report_syscall()
         -> seccomp_permit_syscall()
              -> syscall_get_arguments() -> ARM_ORIG_r0
         -> trace_sys_enter()
              -> syscall_get_arguments() -> ARM_ORIG_r0
         -> audit_syscall_entry(scno, ARM_r0, ARM_r1, ARM_r2, ARM_r3)
				      ^^^^^^
    -> invoke_syscall, reload r0-r6
       -> use r0 here!
             ^^^^

So after a tracer changes r0, seccomp and the tracepoints still read the
old ARM_ORIG_r0, audit reads the new ARM_r0, and the syscall runs with
the new ARM_r0.

riscv and loongarch also keep a copy of the first argument (orig_a0),
but they pass that copy to both the syscall and
syscall_get_arguments(), so they stay consistent.  On arm32 the syscall
runs with a different register than the one seccomp and the tracepoints
read.

The syscall number has a similar copy (abi_syscall), but there the
readers and the dispatcher both use syscall_get_nr(), so only the first
argument is affected.

[Fix]
Copy r0 into ARM_ORIG_r0 after a ptrace register write, but only when
the tracee is stopped before its syscall runs:
 - at a syscall-entry stop or a SECCOMP_RET_TRACE stop, r0 still holds
   the first argument, and seccomp, the tracepoints, audit and the
   syscall itself are about to read it;
 - at a syscall-exit stop, r0 holds the return value (-ERESTARTSYS and
   friends).  ARM_ORIG_r0 must stay, or a restarted syscall would run
   with the error code as its first argument;
 - for any other exception, the entry code sets ARM_ORIG_r0 to ~0 and it
   has no meaning, so it must not be overwritten.

The stop type is the same classification that
ptrace_get_syscall_info_op() uses for PTRACE_GET_SYSCALL_INFO.

audit_syscall_entry() takes the first argument from r0 directly.  Give
it ARM_ORIG_r0, as seccomp and the tracepoints do, so audit describes
the syscall that really runs.

Tested on arm32 with a real gdb in qemu-system-arm: after
"catch syscall getpid" the tracer sets r0 to a value the seccomp filter
rejects.  With this patch the filter refuses the syscall, without it the
syscall runs with the old argument.

Cc: stable@vger.kernel.org
Fixes: bf2c9f986692 ("ARM: 7373/1: add support for the generic syscall.h interface")
Link: https://lore.kernel.org/all/al98jdcleu-6tMDL@willie-the-truck/
Suggested-by: Will Deacon <will@kernel.org>
Link: https://lkml.org/lkml/2020/9/11/1282
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 arch/arm/kernel/ptrace.c | 41 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index ed7a2a87a670..6ec81e5a205e 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -163,6 +163,42 @@ static inline long get_user_reg(struct task_struct *task, int offset)
 	return task_pt_regs(task)->uregs[offset];
 }
 
+/*
+ * Is @target stopped after entering a syscall, but before it has been
+ * dispatched?  This is the same classification that
+ * ptrace_get_syscall_info_op() performs for PTRACE_GET_SYSCALL_INFO.
+ */
+static bool stopped_in_syscall_entry(struct task_struct *target)
+{
+	struct kernel_siginfo *info = target->last_siginfo;
+
+	if (!info)
+		return false;
+
+	if (info->si_code == (SIGTRAP | 0x80))
+		return target->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY;
+
+	return info->si_code == (SIGTRAP | (PTRACE_EVENT_SECCOMP << 8));
+}
+
+static void update_syscall_orig_r0_after_ptrace(struct task_struct *target)
+{
+	struct pt_regs *regs = task_pt_regs(target);
+
+	/*
+	 * ARM_r0 holds the first syscall argument only before the syscall is
+	 * dispatched; seccomp, the syscall tracepoints and the syscall restart
+	 * code all read that argument from ARM_ORIG_r0 instead.  At a syscall
+	 * exit stop ARM_r0 holds the return value and must not be copied, and
+	 * for other exceptions ARM_ORIG_r0 is set to ~0 and carries no
+	 * meaning.
+	 */
+	if (!stopped_in_syscall_entry(target))
+		return;
+
+	regs->ARM_ORIG_r0 = regs->ARM_r0;
+}
+
 /*
  * this routine will put a word on the processes privileged stack.
  * the offset is how far from the base addr as stored in the THREAD.
@@ -180,6 +216,8 @@ put_user_reg(struct task_struct *task, int offset, long data)
 
 	if (valid_user_regs(&newregs)) {
 		regs->uregs[offset] = data;
+		if (offset == 0)
+			update_syscall_orig_r0_after_ptrace(task);
 		ret = 0;
 	}
 
@@ -566,6 +604,7 @@ static int gpr_set(struct task_struct *target,
 		return -EINVAL;
 
 	*task_pt_regs(target) = newregs;
+	update_syscall_orig_r0_after_ptrace(target);
 	return 0;
 }
 
@@ -868,7 +907,7 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs)
 	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
 		trace_sys_enter(regs, scno);
 
-	audit_syscall_entry(scno, regs->ARM_r0, regs->ARM_r1, regs->ARM_r2,
+	audit_syscall_entry(scno, regs->ARM_ORIG_r0, regs->ARM_r1, regs->ARM_r2,
 			    regs->ARM_r3);
 
 	return scno;
-- 
2.34.1

[PATCH v2 2/2] selftests/seccomp: Add tests for syscall argument rewrite

[Jinjie Ruan]

A tracer can change a syscall argument at two stops:

 - at a SECCOMP_RET_TRACE stop, the tracer runs first and the kernel
   checks the filters again after it;
 - at a PTRACE_SYSCALL entry stop, the tracer runs first and seccomp
   runs after it.

In both cases seccomp must see the new argument, because the syscall is
executed with the new value.

The seccomp selftests (TRACE_syscall) change the syscall number and the
return value, but not the arguments.  arm64 has two architecture
specific tests for x0, seccomp_ptrace_x0_bypass.c for the
syscall-entry stop and seccomp_ret_trace_x0_bypass.c for the
SECCOMP_RET_TRACE stop, in tools/testing/selftests/arm64/abi/, but they
run on arm64 only.

These tests call getpid() with a known first argument and install a
filter that rejects the value the tracer writes.  The return value of
getpid() shows which argument the filter saw.  They run on arm32, arm64,
x86-64 and i386, and they cover both stops above.

riscv and loongarch skip: the kernel gives orig_a0, not a0, to both the
syscall and seccomp, so a0 is not the argument register there.

Two tests run on arm32 only:
 - a syscall that returns -ERESTARTSYS and is restarted must run with the
   first argument from the entry, not with the error value;
 - a stop outside a syscall must not change ARM_ORIG_r0, which is ~0
   there.

The first two tests catch the arm32 bug where seccomp sees the old
first argument.

The tests were written with assistance from DeepSeek V4.1 Flash.

Tested on QEMU for arm32 and arm64, and on hardware for x86-64 and i386
(passed/total):
 - arm32: 4/4
 - arm64: 3/3
 - x86-64: 3/3
 - i386: 3/3

No case runs on riscv64 or loongarch64: the test skips there for the
reason above.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 tools/testing/selftests/seccomp/.gitignore    |   1 +
 tools/testing/selftests/seccomp/Makefile      |   2 +-
 .../selftests/seccomp/seccomp_ptrace_args.c   | 463 ++++++++++++++++++
 3 files changed, 465 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/seccomp/seccomp_ptrace_args.c

diff --git a/tools/testing/selftests/seccomp/.gitignore b/tools/testing/selftests/seccomp/.gitignore
index dec678577f9c..ea60bb76136b 100644
--- a/tools/testing/selftests/seccomp/.gitignore
+++ b/tools/testing/selftests/seccomp/.gitignore
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 seccomp_bpf
 seccomp_benchmark
+seccomp_ptrace_args
diff --git a/tools/testing/selftests/seccomp/Makefile b/tools/testing/selftests/seccomp/Makefile
index 584fba487037..4232068a9bd8 100644
--- a/tools/testing/selftests/seccomp/Makefile
+++ b/tools/testing/selftests/seccomp/Makefile
@@ -3,5 +3,5 @@ CFLAGS += -Wl,-no-as-needed -Wall $(KHDR_INCLUDES)
 LDFLAGS += -lpthread
 LDLIBS += -lcap
 
-TEST_GEN_PROGS := seccomp_bpf seccomp_benchmark
+TEST_GEN_PROGS := seccomp_bpf seccomp_benchmark seccomp_ptrace_args
 include ../lib.mk
diff --git a/tools/testing/selftests/seccomp/seccomp_ptrace_args.c b/tools/testing/selftests/seccomp/seccomp_ptrace_args.c
new file mode 100644
index 000000000000..e2e6df54e974
--- /dev/null
+++ b/tools/testing/selftests/seccomp/seccomp_ptrace_args.c
@@ -0,0 +1,463 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Check that the syscall arguments a ptrace tracer rewrites are the ones
+ * seccomp observes.
+ *
+ * A tracer can rewrite the syscall arguments at a SECCOMP_RET_TRACE stop
+ * (the syscall is re-evaluated afterwards) and at a PTRACE_SYSCALL entry
+ * stop (seccomp runs after ptrace on the entry path).  In both cases the
+ * filters and the tracepoint of the syscall that is actually executed have
+ * to see the rewritten value.
+ *
+ * On arm32 the first syscall argument is copied into ARM_ORIG_r0 when the
+ * syscall is entered because r0 is clobbered by the return value on the way
+ * out; seccomp reads that copy through syscall_get_arguments().  A tracer
+ * write to r0 therefore has to be mirrored into ARM_ORIG_r0 by the kernel.
+ * The same register also feeds the syscall restart path, so stops at which
+ * ARM_ORIG_r0 does not carry a syscall argument must be left alone.
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <signal.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#include <sys/prctl.h>
+#include <sys/ptrace.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/user.h>
+#include <sys/wait.h>
+#include <linux/audit.h>
+#include <linux/filter.h>
+#include <linux/ptrace.h>
+#include <linux/seccomp.h>
+#include <elf.h>
+#include <sys/uio.h>
+#if defined(__arm__) || defined(__aarch64__)
+#include <asm/ptrace.h>
+#endif
+
+#include "kselftest_harness.h"
+
+#ifndef __NR_getpid
+#define __NR_getpid 20
+#endif
+
+/* The first argument the tracee passes to getpid(). */
+#define VAL_ORIG	0x11112222UL
+/* The first argument the tracer rewrites it to. */
+#define VAL_NEW		0x33334444UL
+#define ERRNO_MARK	0x7f
+#define TRACE_MARK	0x1001
+
+#if defined(__arm__)
+#define NATIVE_AUDIT_ARCH	AUDIT_ARCH_ARM
+#elif defined(__aarch64__)
+#define NATIVE_AUDIT_ARCH	AUDIT_ARCH_AARCH64
+#elif defined(__x86_64__)
+#define NATIVE_AUDIT_ARCH	AUDIT_ARCH_X86_64
+#elif defined(__i386__)
+#define NATIVE_AUDIT_ARCH	AUDIT_ARCH_I386
+#endif
+
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+#define syscall_arg0	offsetof(struct seccomp_data, args[0])
+#else
+#define syscall_arg0	(offsetof(struct seccomp_data, args[0]) + sizeof(__u32))
+#endif
+
+#if defined(__arm__)
+# define HAVE_ARG0		1
+# define HAVE_ORIG_ARG0		1
+# define ARCH_REGS		struct pt_regs
+# define SYSCALL_ARG0(_regs)	((_regs).ARM_r0)
+# define SYSCALL_ARG0_ORIG(_regs) ((_regs).ARM_ORIG_r0)
+# define SYSCALL_RET_SET(_regs, _val) ((_regs).ARM_r0 = (_val))
+#elif defined(__x86_64__)
+# define HAVE_ARG0		1
+# define ARCH_REGS		struct user_regs_struct
+# define SYSCALL_ARG0(_regs)	((_regs).rdi)
+# define SYSCALL_RET_SET(_regs, _val) ((_regs).rax = (_val))
+#elif defined(__i386__)
+# define HAVE_ARG0		1
+# define ARCH_REGS		struct user_regs_struct
+# define SYSCALL_ARG0(_regs)	((_regs).ebx)
+# define SYSCALL_RET_SET(_regs, _val) ((_regs).eax = (_val))
+#elif defined(__aarch64__)
+# define HAVE_ARG0		1
+# define USE_REGSET		1
+# define ARCH_REGS		struct user_pt_regs
+# define SYSCALL_ARG0(_regs)	((_regs).regs[0])
+# define SYSCALL_RET_SET(_regs, _val) ((_regs).regs[0] = (_val))
+/*
+ * riscv and loongarch are deliberately left out: they hand orig_a0, not
+ * a0, to the syscall and to syscall_get_arguments(), so the register this
+ * test rewrites is not the one the syscall runs with.
+ */
+#endif
+
+#ifdef HAVE_ARG0
+
+/* getpid(VAL_NEW) is refused, getpid(VAL_ORIG) is traced. */
+static const struct sock_filter filter_trace[] = {
+	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, offsetof(struct seccomp_data, arch)),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, NATIVE_AUDIT_ARCH, 0, 7),
+	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, offsetof(struct seccomp_data, nr)),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, __NR_getpid, 0, 6),
+	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, syscall_arg0),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, VAL_NEW, 0, 1),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ERRNO | ERRNO_MARK),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, VAL_ORIG, 0, 2),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_TRACE | TRACE_MARK),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_KILL_PROCESS),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW),
+};
+
+/* getpid(VAL_NEW) is refused, anything else is allowed. */
+static const struct sock_filter filter_allow[] = {
+	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, offsetof(struct seccomp_data, arch)),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, NATIVE_AUDIT_ARCH, 0, 6),
+	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, offsetof(struct seccomp_data, nr)),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, __NR_getpid, 0, 3),
+	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, syscall_arg0),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, VAL_NEW, 0, 1),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ERRNO | ERRNO_MARK),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_KILL_PROCESS),
+};
+
+#define ERESTARTSYS	512
+
+static int install_filter(const struct sock_filter *filter, unsigned int count)
+{
+	struct sock_fprog prog = {
+		.len = (unsigned short)count,
+		.filter = (struct sock_filter *)filter,
+	};
+
+	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0))
+		return -1;
+
+	return prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog, 0, 0);
+}
+
+static int get_regs(pid_t pid, ARCH_REGS *regs)
+{
+#ifdef USE_REGSET
+	struct iovec iov = {
+		.iov_base = regs,
+		.iov_len = sizeof(*regs),
+	};
+
+	return ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov);
+#else
+	return ptrace(PTRACE_GETREGS, pid, NULL, regs);
+#endif
+}
+
+static int set_regs(pid_t pid, ARCH_REGS *regs)
+{
+#ifdef USE_REGSET
+	struct iovec iov = {
+		.iov_base = regs,
+		.iov_len = sizeof(*regs),
+	};
+
+	return ptrace(PTRACE_SETREGSET, pid, NT_PRSTATUS, &iov);
+#else
+	return ptrace(PTRACE_SETREGS, pid, NULL, regs);
+#endif
+}
+
+static long get_syscall_info(pid_t pid, struct ptrace_syscall_info *info)
+{
+	return ptrace(PTRACE_GET_SYSCALL_INFO, pid, sizeof(*info), info);
+}
+
+/* waitpid() with a deadline, so that a kernel bug cannot hang the run. */
+static pid_t wait_for_stop(pid_t pid, int *status, int timeout_ms)
+{
+	while (timeout_ms-- > 0) {
+		pid_t ret = waitpid(pid, status, WNOHANG);
+
+		if (ret == pid)
+			return pid;
+		if (ret < 0 && errno != EINTR)
+			return ret;
+		usleep(1000);
+	}
+
+	return 0;
+}
+
+/* Fork a tracee that stops itself, then calls getpid(VAL_ORIG). */
+static pid_t spawn_tracee(const struct sock_filter *filter, unsigned int count)
+{
+	pid_t tracee = fork();
+
+	if (tracee != 0)
+		return tracee;
+
+	ptrace(PTRACE_TRACEME, 0, NULL, NULL);
+	raise(SIGSTOP);
+	if (install_filter(filter, count))
+		_exit(2);
+	errno = 0;
+	if (syscall(__NR_getpid, VAL_ORIG) == -1 && errno == ERRNO_MARK)
+		_exit(0);	/* seccomp saw VAL_NEW */
+	_exit(1);		/* seccomp saw VAL_ORIG */
+}
+
+static void attach(struct __test_metadata *_metadata, pid_t tracee, long options)
+{
+	int status;
+
+	ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
+		kill(tracee, SIGKILL);
+	}
+	ASSERT_TRUE(WIFSTOPPED(status));
+	ASSERT_EQ(0, ptrace(PTRACE_SETOPTIONS, tracee, NULL,
+			    options | PTRACE_O_EXITKILL));
+}
+
+/*
+ * The tracer rewrites the first argument at the PTRACE_EVENT_SECCOMP stop.
+ * The syscall is re-evaluated after the stop, so the filter runs again with
+ * the new value and refuses it.
+ */
+TEST(seccomp_trace_stop_sees_rewritten_arg)
+{
+	pid_t tracee = spawn_tracee(filter_trace, ARRAY_SIZE(filter_trace));
+	int status;
+
+	ASSERT_GE(tracee, 0);
+	attach(_metadata, tracee,
+	       PTRACE_O_TRACESECCOMP | PTRACE_O_TRACESYSGOOD);
+	ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL, NULL));
+
+	for (;;) {
+		ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
+			kill(tracee, SIGKILL);
+		}
+		if (WIFEXITED(status))
+			break;
+		ASSERT_FALSE(WIFSIGNALED(status));
+
+		if ((status >> 8) ==
+		    (SIGTRAP | (PTRACE_EVENT_SECCOMP << 8))) {
+			ARCH_REGS regs;
+
+			ASSERT_EQ(0, get_regs(tracee, &regs));
+			SYSCALL_ARG0(regs) = VAL_NEW;
+			ASSERT_EQ(0, set_regs(tracee, &regs));
+		}
+		ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL, NULL));
+	}
+
+	EXPECT_EQ(0, WEXITSTATUS(status));
+}
+
+/*
+ * The tracer rewrites the first argument at the syscall-entry stop.  seccomp
+ * runs after ptrace, so the filter has to see the new value.
+ */
+TEST(syscall_entry_stop_sees_rewritten_arg)
+{
+	pid_t tracee = spawn_tracee(filter_allow, ARRAY_SIZE(filter_allow));
+	int status, poked = 0;
+
+	ASSERT_GE(tracee, 0);
+	attach(_metadata, tracee, PTRACE_O_TRACESYSGOOD);
+	ASSERT_EQ(0, ptrace(PTRACE_SYSCALL, tracee, NULL, NULL));
+
+	for (;;) {
+		struct ptrace_syscall_info info;
+		long len;
+
+		ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
+			kill(tracee, SIGKILL);
+		}
+		if (WIFEXITED(status))
+			break;
+		ASSERT_FALSE(WIFSIGNALED(status));
+
+		len = get_syscall_info(tracee, &info);
+		if (len < 0) {
+			kill(tracee, SIGKILL);
+			SKIP(return, "PTRACE_GET_SYSCALL_INFO unsupported");
+		}
+		if (!poked && info.op == PTRACE_SYSCALL_INFO_ENTRY &&
+		    info.entry.nr == __NR_getpid) {
+			ARCH_REGS regs;
+
+			ASSERT_EQ(0, get_regs(tracee, &regs));
+			SYSCALL_ARG0(regs) = VAL_NEW;
+			ASSERT_EQ(0, set_regs(tracee, &regs));
+			poked = 1;
+		}
+		ASSERT_EQ(0, ptrace(PTRACE_SYSCALL, tracee, NULL, NULL));
+	}
+
+	EXPECT_EQ(1, poked);
+	EXPECT_EQ(0, WEXITSTATUS(status));
+}
+
+static void sighandler(int sig)
+{
+	(void)sig;
+}
+
+/*
+ * The tracer makes write() return -ERESTARTSYS at the syscall-exit stop and
+ * delivers a signal with SA_RESTART.  The syscall restarts with the first
+ * argument as it was when the syscall was entered, so the restarted write
+ * still reaches the pipe.  If the return value had replaced the argument,
+ * write() would fail with EBADF instead.
+ */
+TEST(syscall_exit_stop_preserves_arg_for_restart)
+{
+	int pipefd[2];
+	struct sigaction sa = {
+		.sa_handler = sighandler,
+		.sa_flags = SA_RESTART,
+	};
+	pid_t tracee;
+	long last_nr = -1;
+	int status, poked = 0;
+
+	ASSERT_EQ(0, pipe(pipefd));
+
+	tracee = fork();
+	ASSERT_GE(tracee, 0);
+	if (tracee == 0) {
+		close(pipefd[0]);
+		sigaction(SIGUSR1, &sa, NULL);
+		ptrace(PTRACE_TRACEME, 0, NULL, NULL);
+		raise(SIGSTOP);
+		_exit(write(pipefd[1], "x", 1) == 1 ? 0 : 1);
+	}
+	close(pipefd[1]);
+
+	attach(_metadata, tracee, PTRACE_O_TRACESYSGOOD);
+	ASSERT_EQ(0, ptrace(PTRACE_SYSCALL, tracee, NULL, NULL));
+
+	for (;;) {
+		struct ptrace_syscall_info info;
+		long len;
+
+		ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
+			kill(tracee, SIGKILL);
+		}
+		if (WIFEXITED(status))
+			break;
+		ASSERT_FALSE(WIFSIGNALED(status));
+
+		len = get_syscall_info(tracee, &info);
+		if (len < 0) {
+			kill(tracee, SIGKILL);
+			SKIP(return, "PTRACE_GET_SYSCALL_INFO unsupported");
+		}
+		if (info.op == PTRACE_SYSCALL_INFO_ENTRY)
+			last_nr = info.entry.nr;
+		if (!poked && info.op == PTRACE_SYSCALL_INFO_EXIT &&
+		    last_nr == __NR_write) {
+			ARCH_REGS regs;
+
+			ASSERT_EQ(0, get_regs(tracee, &regs));
+			SYSCALL_RET_SET(regs, -ERESTARTSYS);
+			ASSERT_EQ(0, set_regs(tracee, &regs));
+			poked = 1;
+			/* resume into the restart path */
+			ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL,
+					    SIGUSR1));
+			continue;
+		}
+		ASSERT_EQ(0, ptrace(PTRACE_SYSCALL, tracee, NULL, NULL));
+	}
+
+	close(pipefd[0]);
+	EXPECT_EQ(1, poked);
+	EXPECT_EQ(0, WEXITSTATUS(status));
+}
+
+#ifdef HAVE_ORIG_ARG0
+
+/*
+ * The tracer rewrites r0 while the tracee is stopped on a signal delivered
+ * outside of a syscall, where ARM_ORIG_r0 holds the ~0 "no syscall" marker.
+ * The marker must not be replaced by the value the tracer wrote.
+ */
+TEST(non_syscall_stop_keeps_orig_arg)
+{
+	sig_atomic_t *got_signal = mmap(NULL, sizeof(*got_signal),
+					PROT_READ | PROT_WRITE,
+					MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+	struct sigaction sa = {
+		.sa_handler = sighandler,
+	};
+	pid_t tracee;
+	int status;
+	ARCH_REGS regs;
+	unsigned long before;
+
+	ASSERT_TRUE(got_signal != MAP_FAILED);
+	ASSERT_EQ(0, sigaction(SIGUSR1, &sa, NULL));
+
+	tracee = fork();
+	ASSERT_GE(tracee, 0);
+	if (tracee == 0) {
+		ptrace(PTRACE_TRACEME, 0, NULL, NULL);
+		raise(SIGSTOP);
+		/*
+		 * Spin in user mode so that the signal is not delivered on
+		 * the return path of a syscall.
+		 */
+		while (!__atomic_load_n(got_signal, __ATOMIC_RELAXED))
+			;
+		_exit(0);
+	}
+
+	__atomic_store_n(got_signal, 0, __ATOMIC_RELAXED);
+	attach(_metadata, tracee, 0);
+	ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL, NULL));
+	usleep(200000);
+	ASSERT_EQ(0, kill(tracee, SIGUSR1));
+
+	ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
+		kill(tracee, SIGKILL);
+	}
+	ASSERT_TRUE(WIFSTOPPED(status));
+	ASSERT_EQ(SIGUSR1, WSTOPSIG(status));
+
+	ASSERT_EQ(0, get_regs(tracee, &regs));
+	before = SYSCALL_ARG0_ORIG(regs);
+	SYSCALL_ARG0(regs) = 0xdeadbeefUL;
+	ASSERT_EQ(0, set_regs(tracee, &regs));
+	ASSERT_EQ(0, get_regs(tracee, &regs));
+
+	if (before == 0xffffffffUL)
+		EXPECT_EQ(before, (unsigned long)SYSCALL_ARG0_ORIG(regs));
+
+	__atomic_store_n(got_signal, 1, __ATOMIC_RELAXED);
+	ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL, SIGUSR1));
+	ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
+		kill(tracee, SIGKILL);
+	}
+}
+
+#endif /* HAVE_ORIG_ARG0 */
+
+#else /* !HAVE_ARG0 */
+
+TEST(unsupported_arch)
+{
+	SKIP(return, "no syscall argument 0 accessor for this architecture");
+}
+
+#endif /* HAVE_ARG0 */
+
+TEST_HARNESS_MAIN
-- 
2.34.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 2/2] selftests/seccomp: Add tests for syscall argument rewrite

[Jinjie Ruan]

A tracer can change a syscall argument at two stops:

 - at a SECCOMP_RET_TRACE stop, the tracer runs first and the kernel
   checks the filters again after it;
 - at a PTRACE_SYSCALL entry stop, the tracer runs first and seccomp
   runs after it.

In both cases seccomp must see the new argument, because the syscall is
executed with the new value.

The seccomp selftests (TRACE_syscall) change the syscall number and the
return value, but not the arguments.  arm64 has two architecture
specific tests for x0, seccomp_ptrace_x0_bypass.c for the
syscall-entry stop and seccomp_ret_trace_x0_bypass.c for the
SECCOMP_RET_TRACE stop, in tools/testing/selftests/arm64/abi/, but they
run on arm64 only.

These tests call getpid() with a known first argument and install a
filter that rejects the value the tracer writes.  The return value of
getpid() shows which argument the filter saw.  They run on arm32, arm64,
x86-64 and i386, and they cover both stops above.

riscv and loongarch skip: the kernel gives orig_a0, not a0, to both the
syscall and seccomp, so a0 is not the argument register there.

Two tests run on arm32 only:
 - a syscall that returns -ERESTARTSYS and is restarted must run with the
   first argument from the entry, not with the error value;
 - a stop outside a syscall must not change ARM_ORIG_r0, which is ~0
   there.

The first two tests catch the arm32 bug where seccomp sees the old
first argument.

The tests were written with assistance from DeepSeek V4.1 Flash.

Tested on QEMU for arm32 and arm64, and on hardware for x86-64 and i386
(passed/total):
 - arm32: 4/4
 - arm64: 3/3
 - x86-64: 3/3
 - i386: 3/3

No case runs on riscv64 or loongarch64: the test skips there for the
reason above.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 tools/testing/selftests/seccomp/.gitignore    |   1 +
 tools/testing/selftests/seccomp/Makefile      |   2 +-
 .../selftests/seccomp/seccomp_ptrace_args.c   | 463 ++++++++++++++++++
 3 files changed, 465 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/seccomp/seccomp_ptrace_args.c

diff --git a/tools/testing/selftests/seccomp/.gitignore b/tools/testing/selftests/seccomp/.gitignore
index dec678577f9c..ea60bb76136b 100644
--- a/tools/testing/selftests/seccomp/.gitignore
+++ b/tools/testing/selftests/seccomp/.gitignore
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 seccomp_bpf
 seccomp_benchmark
+seccomp_ptrace_args
diff --git a/tools/testing/selftests/seccomp/Makefile b/tools/testing/selftests/seccomp/Makefile
index 584fba487037..4232068a9bd8 100644
--- a/tools/testing/selftests/seccomp/Makefile
+++ b/tools/testing/selftests/seccomp/Makefile
@@ -3,5 +3,5 @@ CFLAGS += -Wl,-no-as-needed -Wall $(KHDR_INCLUDES)
 LDFLAGS += -lpthread
 LDLIBS += -lcap
 
-TEST_GEN_PROGS := seccomp_bpf seccomp_benchmark
+TEST_GEN_PROGS := seccomp_bpf seccomp_benchmark seccomp_ptrace_args
 include ../lib.mk
diff --git a/tools/testing/selftests/seccomp/seccomp_ptrace_args.c b/tools/testing/selftests/seccomp/seccomp_ptrace_args.c
new file mode 100644
index 000000000000..e2e6df54e974
--- /dev/null
+++ b/tools/testing/selftests/seccomp/seccomp_ptrace_args.c
@@ -0,0 +1,463 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Check that the syscall arguments a ptrace tracer rewrites are the ones
+ * seccomp observes.
+ *
+ * A tracer can rewrite the syscall arguments at a SECCOMP_RET_TRACE stop
+ * (the syscall is re-evaluated afterwards) and at a PTRACE_SYSCALL entry
+ * stop (seccomp runs after ptrace on the entry path).  In both cases the
+ * filters and the tracepoint of the syscall that is actually executed have
+ * to see the rewritten value.
+ *
+ * On arm32 the first syscall argument is copied into ARM_ORIG_r0 when the
+ * syscall is entered because r0 is clobbered by the return value on the way
+ * out; seccomp reads that copy through syscall_get_arguments().  A tracer
+ * write to r0 therefore has to be mirrored into ARM_ORIG_r0 by the kernel.
+ * The same register also feeds the syscall restart path, so stops at which
+ * ARM_ORIG_r0 does not carry a syscall argument must be left alone.
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <signal.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#include <sys/prctl.h>
+#include <sys/ptrace.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/user.h>
+#include <sys/wait.h>
+#include <linux/audit.h>
+#include <linux/filter.h>
+#include <linux/ptrace.h>
+#include <linux/seccomp.h>
+#include <elf.h>
+#include <sys/uio.h>
+#if defined(__arm__) || defined(__aarch64__)
+#include <asm/ptrace.h>
+#endif
+
+#include "kselftest_harness.h"
+
+#ifndef __NR_getpid
+#define __NR_getpid 20
+#endif
+
+/* The first argument the tracee passes to getpid(). */
+#define VAL_ORIG	0x11112222UL
+/* The first argument the tracer rewrites it to. */
+#define VAL_NEW		0x33334444UL
+#define ERRNO_MARK	0x7f
+#define TRACE_MARK	0x1001
+
+#if defined(__arm__)
+#define NATIVE_AUDIT_ARCH	AUDIT_ARCH_ARM
+#elif defined(__aarch64__)
+#define NATIVE_AUDIT_ARCH	AUDIT_ARCH_AARCH64
+#elif defined(__x86_64__)
+#define NATIVE_AUDIT_ARCH	AUDIT_ARCH_X86_64
+#elif defined(__i386__)
+#define NATIVE_AUDIT_ARCH	AUDIT_ARCH_I386
+#endif
+
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+#define syscall_arg0	offsetof(struct seccomp_data, args[0])
+#else
+#define syscall_arg0	(offsetof(struct seccomp_data, args[0]) + sizeof(__u32))
+#endif
+
+#if defined(__arm__)
+# define HAVE_ARG0		1
+# define HAVE_ORIG_ARG0		1
+# define ARCH_REGS		struct pt_regs
+# define SYSCALL_ARG0(_regs)	((_regs).ARM_r0)
+# define SYSCALL_ARG0_ORIG(_regs) ((_regs).ARM_ORIG_r0)
+# define SYSCALL_RET_SET(_regs, _val) ((_regs).ARM_r0 = (_val))
+#elif defined(__x86_64__)
+# define HAVE_ARG0		1
+# define ARCH_REGS		struct user_regs_struct
+# define SYSCALL_ARG0(_regs)	((_regs).rdi)
+# define SYSCALL_RET_SET(_regs, _val) ((_regs).rax = (_val))
+#elif defined(__i386__)
+# define HAVE_ARG0		1
+# define ARCH_REGS		struct user_regs_struct
+# define SYSCALL_ARG0(_regs)	((_regs).ebx)
+# define SYSCALL_RET_SET(_regs, _val) ((_regs).eax = (_val))
+#elif defined(__aarch64__)
+# define HAVE_ARG0		1
+# define USE_REGSET		1
+# define ARCH_REGS		struct user_pt_regs
+# define SYSCALL_ARG0(_regs)	((_regs).regs[0])
+# define SYSCALL_RET_SET(_regs, _val) ((_regs).regs[0] = (_val))
+/*
+ * riscv and loongarch are deliberately left out: they hand orig_a0, not
+ * a0, to the syscall and to syscall_get_arguments(), so the register this
+ * test rewrites is not the one the syscall runs with.
+ */
+#endif
+
+#ifdef HAVE_ARG0
+
+/* getpid(VAL_NEW) is refused, getpid(VAL_ORIG) is traced. */
+static const struct sock_filter filter_trace[] = {
+	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, offsetof(struct seccomp_data, arch)),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, NATIVE_AUDIT_ARCH, 0, 7),
+	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, offsetof(struct seccomp_data, nr)),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, __NR_getpid, 0, 6),
+	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, syscall_arg0),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, VAL_NEW, 0, 1),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ERRNO | ERRNO_MARK),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, VAL_ORIG, 0, 2),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_TRACE | TRACE_MARK),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_KILL_PROCESS),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW),
+};
+
+/* getpid(VAL_NEW) is refused, anything else is allowed. */
+static const struct sock_filter filter_allow[] = {
+	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, offsetof(struct seccomp_data, arch)),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, NATIVE_AUDIT_ARCH, 0, 6),
+	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, offsetof(struct seccomp_data, nr)),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, __NR_getpid, 0, 3),
+	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, syscall_arg0),
+	BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, VAL_NEW, 0, 1),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ERRNO | ERRNO_MARK),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_ALLOW),
+	BPF_STMT(BPF_RET | BPF_K, SECCOMP_RET_KILL_PROCESS),
+};
+
+#define ERESTARTSYS	512
+
+static int install_filter(const struct sock_filter *filter, unsigned int count)
+{
+	struct sock_fprog prog = {
+		.len = (unsigned short)count,
+		.filter = (struct sock_filter *)filter,
+	};
+
+	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0))
+		return -1;
+
+	return prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog, 0, 0);
+}
+
+static int get_regs(pid_t pid, ARCH_REGS *regs)
+{
+#ifdef USE_REGSET
+	struct iovec iov = {
+		.iov_base = regs,
+		.iov_len = sizeof(*regs),
+	};
+
+	return ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, &iov);
+#else
+	return ptrace(PTRACE_GETREGS, pid, NULL, regs);
+#endif
+}
+
+static int set_regs(pid_t pid, ARCH_REGS *regs)
+{
+#ifdef USE_REGSET
+	struct iovec iov = {
+		.iov_base = regs,
+		.iov_len = sizeof(*regs),
+	};
+
+	return ptrace(PTRACE_SETREGSET, pid, NT_PRSTATUS, &iov);
+#else
+	return ptrace(PTRACE_SETREGS, pid, NULL, regs);
+#endif
+}
+
+static long get_syscall_info(pid_t pid, struct ptrace_syscall_info *info)
+{
+	return ptrace(PTRACE_GET_SYSCALL_INFO, pid, sizeof(*info), info);
+}
+
+/* waitpid() with a deadline, so that a kernel bug cannot hang the run. */
+static pid_t wait_for_stop(pid_t pid, int *status, int timeout_ms)
+{
+	while (timeout_ms-- > 0) {
+		pid_t ret = waitpid(pid, status, WNOHANG);
+
+		if (ret == pid)
+			return pid;
+		if (ret < 0 && errno != EINTR)
+			return ret;
+		usleep(1000);
+	}
+
+	return 0;
+}
+
+/* Fork a tracee that stops itself, then calls getpid(VAL_ORIG). */
+static pid_t spawn_tracee(const struct sock_filter *filter, unsigned int count)
+{
+	pid_t tracee = fork();
+
+	if (tracee != 0)
+		return tracee;
+
+	ptrace(PTRACE_TRACEME, 0, NULL, NULL);
+	raise(SIGSTOP);
+	if (install_filter(filter, count))
+		_exit(2);
+	errno = 0;
+	if (syscall(__NR_getpid, VAL_ORIG) == -1 && errno == ERRNO_MARK)
+		_exit(0);	/* seccomp saw VAL_NEW */
+	_exit(1);		/* seccomp saw VAL_ORIG */
+}
+
+static void attach(struct __test_metadata *_metadata, pid_t tracee, long options)
+{
+	int status;
+
+	ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
+		kill(tracee, SIGKILL);
+	}
+	ASSERT_TRUE(WIFSTOPPED(status));
+	ASSERT_EQ(0, ptrace(PTRACE_SETOPTIONS, tracee, NULL,
+			    options | PTRACE_O_EXITKILL));
+}
+
+/*
+ * The tracer rewrites the first argument at the PTRACE_EVENT_SECCOMP stop.
+ * The syscall is re-evaluated after the stop, so the filter runs again with
+ * the new value and refuses it.
+ */
+TEST(seccomp_trace_stop_sees_rewritten_arg)
+{
+	pid_t tracee = spawn_tracee(filter_trace, ARRAY_SIZE(filter_trace));
+	int status;
+
+	ASSERT_GE(tracee, 0);
+	attach(_metadata, tracee,
+	       PTRACE_O_TRACESECCOMP | PTRACE_O_TRACESYSGOOD);
+	ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL, NULL));
+
+	for (;;) {
+		ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
+			kill(tracee, SIGKILL);
+		}
+		if (WIFEXITED(status))
+			break;
+		ASSERT_FALSE(WIFSIGNALED(status));
+
+		if ((status >> 8) ==
+		    (SIGTRAP | (PTRACE_EVENT_SECCOMP << 8))) {
+			ARCH_REGS regs;
+
+			ASSERT_EQ(0, get_regs(tracee, &regs));
+			SYSCALL_ARG0(regs) = VAL_NEW;
+			ASSERT_EQ(0, set_regs(tracee, &regs));
+		}
+		ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL, NULL));
+	}
+
+	EXPECT_EQ(0, WEXITSTATUS(status));
+}
+
+/*
+ * The tracer rewrites the first argument at the syscall-entry stop.  seccomp
+ * runs after ptrace, so the filter has to see the new value.
+ */
+TEST(syscall_entry_stop_sees_rewritten_arg)
+{
+	pid_t tracee = spawn_tracee(filter_allow, ARRAY_SIZE(filter_allow));
+	int status, poked = 0;
+
+	ASSERT_GE(tracee, 0);
+	attach(_metadata, tracee, PTRACE_O_TRACESYSGOOD);
+	ASSERT_EQ(0, ptrace(PTRACE_SYSCALL, tracee, NULL, NULL));
+
+	for (;;) {
+		struct ptrace_syscall_info info;
+		long len;
+
+		ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
+			kill(tracee, SIGKILL);
+		}
+		if (WIFEXITED(status))
+			break;
+		ASSERT_FALSE(WIFSIGNALED(status));
+
+		len = get_syscall_info(tracee, &info);
+		if (len < 0) {
+			kill(tracee, SIGKILL);
+			SKIP(return, "PTRACE_GET_SYSCALL_INFO unsupported");
+		}
+		if (!poked && info.op == PTRACE_SYSCALL_INFO_ENTRY &&
+		    info.entry.nr == __NR_getpid) {
+			ARCH_REGS regs;
+
+			ASSERT_EQ(0, get_regs(tracee, &regs));
+			SYSCALL_ARG0(regs) = VAL_NEW;
+			ASSERT_EQ(0, set_regs(tracee, &regs));
+			poked = 1;
+		}
+		ASSERT_EQ(0, ptrace(PTRACE_SYSCALL, tracee, NULL, NULL));
+	}
+
+	EXPECT_EQ(1, poked);
+	EXPECT_EQ(0, WEXITSTATUS(status));
+}
+
+static void sighandler(int sig)
+{
+	(void)sig;
+}
+
+/*
+ * The tracer makes write() return -ERESTARTSYS at the syscall-exit stop and
+ * delivers a signal with SA_RESTART.  The syscall restarts with the first
+ * argument as it was when the syscall was entered, so the restarted write
+ * still reaches the pipe.  If the return value had replaced the argument,
+ * write() would fail with EBADF instead.
+ */
+TEST(syscall_exit_stop_preserves_arg_for_restart)
+{
+	int pipefd[2];
+	struct sigaction sa = {
+		.sa_handler = sighandler,
+		.sa_flags = SA_RESTART,
+	};
+	pid_t tracee;
+	long last_nr = -1;
+	int status, poked = 0;
+
+	ASSERT_EQ(0, pipe(pipefd));
+
+	tracee = fork();
+	ASSERT_GE(tracee, 0);
+	if (tracee == 0) {
+		close(pipefd[0]);
+		sigaction(SIGUSR1, &sa, NULL);
+		ptrace(PTRACE_TRACEME, 0, NULL, NULL);
+		raise(SIGSTOP);
+		_exit(write(pipefd[1], "x", 1) == 1 ? 0 : 1);
+	}
+	close(pipefd[1]);
+
+	attach(_metadata, tracee, PTRACE_O_TRACESYSGOOD);
+	ASSERT_EQ(0, ptrace(PTRACE_SYSCALL, tracee, NULL, NULL));
+
+	for (;;) {
+		struct ptrace_syscall_info info;
+		long len;
+
+		ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
+			kill(tracee, SIGKILL);
+		}
+		if (WIFEXITED(status))
+			break;
+		ASSERT_FALSE(WIFSIGNALED(status));
+
+		len = get_syscall_info(tracee, &info);
+		if (len < 0) {
+			kill(tracee, SIGKILL);
+			SKIP(return, "PTRACE_GET_SYSCALL_INFO unsupported");
+		}
+		if (info.op == PTRACE_SYSCALL_INFO_ENTRY)
+			last_nr = info.entry.nr;
+		if (!poked && info.op == PTRACE_SYSCALL_INFO_EXIT &&
+		    last_nr == __NR_write) {
+			ARCH_REGS regs;
+
+			ASSERT_EQ(0, get_regs(tracee, &regs));
+			SYSCALL_RET_SET(regs, -ERESTARTSYS);
+			ASSERT_EQ(0, set_regs(tracee, &regs));
+			poked = 1;
+			/* resume into the restart path */
+			ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL,
+					    SIGUSR1));
+			continue;
+		}
+		ASSERT_EQ(0, ptrace(PTRACE_SYSCALL, tracee, NULL, NULL));
+	}
+
+	close(pipefd[0]);
+	EXPECT_EQ(1, poked);
+	EXPECT_EQ(0, WEXITSTATUS(status));
+}
+
+#ifdef HAVE_ORIG_ARG0
+
+/*
+ * The tracer rewrites r0 while the tracee is stopped on a signal delivered
+ * outside of a syscall, where ARM_ORIG_r0 holds the ~0 "no syscall" marker.
+ * The marker must not be replaced by the value the tracer wrote.
+ */
+TEST(non_syscall_stop_keeps_orig_arg)
+{
+	sig_atomic_t *got_signal = mmap(NULL, sizeof(*got_signal),
+					PROT_READ | PROT_WRITE,
+					MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+	struct sigaction sa = {
+		.sa_handler = sighandler,
+	};
+	pid_t tracee;
+	int status;
+	ARCH_REGS regs;
+	unsigned long before;
+
+	ASSERT_TRUE(got_signal != MAP_FAILED);
+	ASSERT_EQ(0, sigaction(SIGUSR1, &sa, NULL));
+
+	tracee = fork();
+	ASSERT_GE(tracee, 0);
+	if (tracee == 0) {
+		ptrace(PTRACE_TRACEME, 0, NULL, NULL);
+		raise(SIGSTOP);
+		/*
+		 * Spin in user mode so that the signal is not delivered on
+		 * the return path of a syscall.
+		 */
+		while (!__atomic_load_n(got_signal, __ATOMIC_RELAXED))
+			;
+		_exit(0);
+	}
+
+	__atomic_store_n(got_signal, 0, __ATOMIC_RELAXED);
+	attach(_metadata, tracee, 0);
+	ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL, NULL));
+	usleep(200000);
+	ASSERT_EQ(0, kill(tracee, SIGUSR1));
+
+	ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
+		kill(tracee, SIGKILL);
+	}
+	ASSERT_TRUE(WIFSTOPPED(status));
+	ASSERT_EQ(SIGUSR1, WSTOPSIG(status));
+
+	ASSERT_EQ(0, get_regs(tracee, &regs));
+	before = SYSCALL_ARG0_ORIG(regs);
+	SYSCALL_ARG0(regs) = 0xdeadbeefUL;
+	ASSERT_EQ(0, set_regs(tracee, &regs));
+	ASSERT_EQ(0, get_regs(tracee, &regs));
+
+	if (before == 0xffffffffUL)
+		EXPECT_EQ(before, (unsigned long)SYSCALL_ARG0_ORIG(regs));
+
+	__atomic_store_n(got_signal, 1, __ATOMIC_RELAXED);
+	ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL, SIGUSR1));
+	ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
+		kill(tracee, SIGKILL);
+	}
+}
+
+#endif /* HAVE_ORIG_ARG0 */
+
+#else /* !HAVE_ARG0 */
+
+TEST(unsupported_arch)
+{
+	SKIP(return, "no syscall argument 0 accessor for this architecture");
+}
+
+#endif /* HAVE_ARG0 */
+
+TEST_HARNESS_MAIN
-- 
2.34.1

Re: [PATCH v2 1/2] ARM: ptrace: Fix seccomp seeing stale syscall arguments

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] ARM: ptrace: missing syscall-entry match in stopped_in_syscall_entry()
- [High] ARM: ptrace: ARM_ORIG_r0 desync in put_user_reg() via PTRACE_POKEUSR
- [High] ARM: ptrace: ARM_ORIG_r0 clobbered in update_syscall_orig_r0_after_ptrace()
--

commit 83d7d1ba6dcc0fb6cba778b5030dfb3da61a6531
Author: Jinjie Ruan <ruanjinjie@huawei.com>

ARM: ptrace: Fix seccomp seeing stale syscall arguments

This commit synchronizes ARM_ORIG_r0 with ARM_r0 after ptrace register writes
when the tracee is stopped in syscall entry or SECCOMP_RET_TRACE, and updates
audit_syscall_entry() to read ARM_ORIG_r0.

Link: https://lore.kernel.org/all/al98jdcleu-6tMDL@willie-the-truck/
Link: https://lkml.org/lkml/2020/9/11/1282

> diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
> index ed7a2a87a6707..6ec81e5a205e7 100644
> --- a/arch/arm/kernel/ptrace.c
> +++ b/arch/arm/kernel/ptrace.c
> @@ -163,6 +163,42 @@ static inline long get_user_reg(struct task_struct *task, int offset)
>  	return task_pt_regs(task)->uregs[offset];
>  }
>  
> +/*
> + * Is @target stopped after entering a syscall, but before it has been
> + * dispatched?  This is the same classification that
> + * ptrace_get_syscall_info_op() performs for PTRACE_GET_SYSCALL_INFO.
> + */
> +static bool stopped_in_syscall_entry(struct task_struct *target)
> +{
> +	struct kernel_siginfo *info = target->last_siginfo;
> +
> +	if (!info)
> +		return false;
> +
> +	if (info->si_code == (SIGTRAP | 0x80))
> +		return target->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY;

[Severity: High]
What happens when a tracer stops a tracee at syscall entry via PTRACE_SYSCALL
without enabling PTRACE_O_TRACESYSGOOD?

In include/linux/ptrace.h:ptrace_report_syscall(), bit 0x80 is only set in
si_code when PT_TRACESYSGOOD is enabled:

	signr = ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0),
			      message);

When PTRACE_O_TRACESYSGOOD is not set, info->si_code is plain SIGTRAP while
target->ptrace_message is PTRACE_EVENTMSG_SYSCALL_ENTRY. Checking for
info->si_code == (SIGTRAP | 0x80) causes stopped_in_syscall_entry() to return
false in that case, leaving regs->ARM_ORIG_r0 unsynchronized when the tracer
updates ARM_r0.

When the tracee resumes in syscall_trace_enter(), seccomp_permit_syscall(),
trace_sys_enter(), and audit_syscall_entry() (which this patch switches from
regs->ARM_r0 to regs->ARM_ORIG_r0) will all observe the stale
regs->ARM_ORIG_r0 while invoke_syscall runs with the modified regs->ARM_r0.

Should this check (info->si_code & ~0x80) == SIGTRAP, matching arm64's
stopped_in_syscall_entry()?

> +
> +	return info->si_code == (SIGTRAP | (PTRACE_EVENT_SECCOMP << 8));
> +}
> +
> +static void update_syscall_orig_r0_after_ptrace(struct task_struct *target)
> +{
> +	struct pt_regs *regs = task_pt_regs(target);
> +
> +	/*
> +	 * ARM_r0 holds the first syscall argument only before the syscall is
> +	 * dispatched; seccomp, the syscall tracepoints and the syscall restart
> +	 * code all read that argument from ARM_ORIG_r0 instead.  At a syscall
> +	 * exit stop ARM_r0 holds the return value and must not be copied, and
> +	 * for other exceptions ARM_ORIG_r0 is set to ~0 and carries no
> +	 * meaning.
> +	 */
> +	if (!stopped_in_syscall_entry(target))
> +		return;
> +
> +	regs->ARM_ORIG_r0 = regs->ARM_r0;
> +}

[Severity: High]
Can update_syscall_orig_r0_after_ptrace() clobber the saved first argument
when a tracer skips a syscall?

On 32-bit ARM, skipping a syscall at a syscall-entry or PTRACE_EVENT_SECCOMP
stop sets task_thread_info(target)->abi_syscall to -1 via PTRACE_SET_SYSCALL
in arch_ptrace() or via syscall_set_nr():

	if (nr == -1) {
		task_thread_info(task)->abi_syscall = -1;
		syscall_set_return_value(task, regs, -ENOSYS, 0);
		return;
	}

Once the syscall number is -1, regs->ARM_r0 holds the return value (such as
-ENOSYS or a tracer-supplied return value), while regs->ARM_ORIG_r0 preserves
the original first argument. Because update_syscall_orig_r0_after_ptrace()
does not check whether syscall_get_nr(target, regs) == -1 (unlike arm64,
which returns early when regs->syscallno == NO_SYSCALL), any subsequent
GPR write via put_user_reg() or gpr_set() overwrites regs->ARM_ORIG_r0 with
the return value.

That corrupts the first argument reported by syscall_get_arguments() (used by
PTRACE_GET_SYSCALL_INFO, trace_sys_enter(), and seccomp) and
audit_syscall_entry(), breaks syscall_rollback() if a later seccomp filter
triggers SECCOMP_RET_TRAP or SECCOMP_RET_KILL_PROCESS, and causes do_signal()
on the __sys_trace_return_nosave -> ret_slow_syscall path to copy an injected
-ERESTART* return value into ARM_r0 as the restarted syscall's first argument.

Conversely, in arch_ptrace(), PTRACE_SET_SYSCALL does not call
update_syscall_orig_r0_after_ptrace(child) when changing abi_syscall from -1
to a valid syscall number, leaving ARM_ORIG_r0 and ARM_r0 out of sync if
syscall_set_nr() had previously set ARM_r0 to -ENOSYS.

Should update_syscall_orig_r0_after_ptrace() return early when
syscall_get_nr(target, regs) == -1, and should PTRACE_SET_SYSCALL call
update_syscall_orig_r0_after_ptrace(child)?

> +
>  /*
>   * this routine will put a word on the processes privileged stack.
>   * the offset is how far from the base addr as stored in the THREAD.
> @@ -180,6 +216,8 @@ put_user_reg(struct task_struct *task, int offset, long data)
>  
>  	if (valid_user_regs(&newregs)) {
>  		regs->uregs[offset] = data;
> +		if (offset == 0)
> +			update_syscall_orig_r0_after_ptrace(task);
>  		ret = 0;
>  	}

[Severity: High]
Could a tracer still desynchronize regs->ARM_ORIG_r0 and regs->ARM_r0 via
PTRACE_POKEUSR in put_user_reg()?

On 32-bit ARM, struct pt_regs exposes all 18 words (uregs[0..17]) to
ptrace_write_user() -> put_user_reg(), where offset 0 is ARM_r0 and offset 17
is ARM_ORIG_r0. At the same time, invoke_syscall in
arch/arm/kernel/entry-header.S dispatches the syscall by reloading r0-r6 from
regs->ARM_r0:

	add	r1, sp, #S_R0 + S_OFF		@ pointer to regs
	ldmiacc	r1, {r0 - r6}			@ reload r0-r6

Because put_user_reg() only calls update_syscall_orig_r0_after_ptrace(task)
when offset == 0, a tracer stopped at syscall entry or PTRACE_EVENT_SECCOMP
can poke offset 17 (ARM_ORIG_r0) directly (or modify
target->last_siginfo->si_code via PTRACE_SETSIGINFO before writing ARM_r0).
In that case, seccomp_permit_syscall(), trace_sys_enter(), and now
audit_syscall_entry() all inspect the value written to regs->ARM_ORIG_r0,
while invoke_syscall executes the syscall with regs->ARM_r0.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030063630.1218302-1-ruanjinjie@huawei.com?part=1

Re: [PATCH v2 2/2] selftests/seccomp: Add tests for syscall argument rewrite

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] selftests/seccomp: broken non_syscall_stop_keeps_orig_arg() synchronization
- [Medium] selftests/seccomp: SIGUSR1 suppressed at signal-delivery stop in restart test
- [Medium] selftests/seccomp: commit message mismatch on arm32-only test guard
- [Medium] selftests/seccomp: seccomp_ptrace_args duplicates seccomp_bpf helpers
--

commit a4698a2d9cf8e2f019613efd5e196f42fdab8a8c
Author: Jinjie Ruan <ruanjinjie@huawei.com>

selftests/seccomp: Add tests for syscall argument rewrite

This commit adds a standalone seccomp_ptrace_args selftest to verify that
seccomp filters observe syscall arguments rewritten by a ptrace tracer at
SECCOMP_RET_TRACE and PTRACE_SYSCALL entry stops, as well as testing
syscall restart argument preservation and non-syscall stops on arm32.

>     selftests/seccomp: Add tests for syscall argument rewrite
>     
>     A tracer can change a syscall argument at two stops:
>     
>      - at a SECCOMP_RET_TRACE stop, the tracer runs first and the kernel
>        checks the filters again after it;
>      - at a PTRACE_SYSCALL entry stop, the tracer runs first and seccomp
>        runs after it.
>     
>     In both cases seccomp must see the new argument, because the syscall is
>     executed with the new value.
>     
>     The seccomp selftests (TRACE_syscall) change the syscall number and the
>     return value, but not the arguments.  arm64 has two architecture
>     specific tests for x0, seccomp_ptrace_x0_bypass.c for the
>     syscall-entry stop and seccomp_ret_trace_x0_bypass.c for the
>     SECCOMP_RET_TRACE stop, in tools/testing/selftests/arm64/abi/, but they
>     run on arm64 only.
>     
>     These tests call getpid() with a known first argument and install a
>     filter that rejects the value the tracer writes.  The return value of
>     getpid() shows which argument the filter saw.  They run on arm32, arm64,
>     x86-64 and i386, and they cover both stops above.
>     
>     riscv and loongarch skip: the kernel gives orig_a0, not a0, to both the
>     syscall and seccomp, so a0 is not the argument register there.
>     
>     Two tests run on arm32 only:
>      - a syscall that returns -ERESTARTSYS and is restarted must run with the
>        first argument from the entry, not with the error value;
>      - a stop outside a syscall must not change ARM_ORIG_r0, which is ~0
>        there.
>     
>     The first two tests catch the arm32 bug where seccomp sees the old
>     first argument.
>     
>     The tests were written with assistance from DeepSeek V4.1 Flash.
>     
>     Tested on QEMU for arm32 and arm64, and on hardware for x86-64 and i386
>     (passed/total):
>      - arm32: 4/4
>      - arm64: 3/3
>      - x86-64: 3/3
>      - i386: 3/3
>     
>     No case runs on riscv64 or loongarch64: the test skips there for the
>     reason above.
>     
>     Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>

[Severity: Medium]
The commit message states that two tests run on arm32 only (the
-ERESTARTSYS restart test and the stop outside a syscall test).

However, in seccomp_ptrace_args.c,
TEST(syscall_exit_stop_preserves_arg_for_restart) is defined under
HAVE_ARG0 rather than HAVE_ORIG_ARG0, so it is built and executed on
arm64, x86-64, and i386 as well (which matches the 3/3 counts reported in
the commit message for those architectures, whereas only
TEST(non_syscall_stop_keeps_orig_arg) is guarded by HAVE_ORIG_ARG0).

Should the commit message be updated to match the #ifdef guards in the
test, or was syscall_exit_stop_preserves_arg_for_restart intended to be
inside HAVE_ORIG_ARG0?

> diff --git a/tools/testing/selftests/seccomp/Makefile b/tools/testing/selftests/seccomp/Makefile
> index 584fba4870372..4232068a9bd8e 100644
> --- a/tools/testing/selftests/seccomp/Makefile
> +++ b/tools/testing/selftests/seccomp/Makefile
> @@ -3,5 +3,5 @@ CFLAGS += -Wl,-no-as-needed -Wall $(KHDR_INCLUDES)
>  LDFLAGS += -lpthread
>  LDLIBS += -lcap
>  
> -TEST_GEN_PROGS := seccomp_bpf seccomp_benchmark
> +TEST_GEN_PROGS := seccomp_bpf seccomp_benchmark seccomp_ptrace_args
>  include ../lib.mk
> diff --git a/tools/testing/selftests/seccomp/seccomp_ptrace_args.c b/tools/testing/selftests/seccomp/seccomp_ptrace_args.c
> new file mode 100644
> index 0000000000000..e2e6df54e9749
> --- /dev/null
> +++ b/tools/testing/selftests/seccomp/seccomp_ptrace_args.c
[ ... ]
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +#define syscall_arg0	offsetof(struct seccomp_data, args[0])
> +#else
> +#define syscall_arg0	(offsetof(struct seccomp_data, args[0]) + sizeof(__u32))
> +#endif
> +
> +#if defined(__arm__)
> +# define HAVE_ARG0		1
> +# define HAVE_ORIG_ARG0		1
> +# define ARCH_REGS		struct pt_regs
> +# define SYSCALL_ARG0(_regs)	((_regs).ARM_r0)
> +# define SYSCALL_ARG0_ORIG(_regs) ((_regs).ARM_ORIG_r0)
> +# define SYSCALL_RET_SET(_regs, _val) ((_regs).ARM_r0 = (_val))
> +#elif defined(__x86_64__)
> +# define HAVE_ARG0		1
> +# define ARCH_REGS		struct user_regs_struct
> +# define SYSCALL_ARG0(_regs)	((_regs).rdi)
> +# define SYSCALL_RET_SET(_regs, _val) ((_regs).rax = (_val))
> +#elif defined(__i386__)
> +# define HAVE_ARG0		1
> +# define ARCH_REGS		struct user_regs_struct
> +# define SYSCALL_ARG0(_regs)	((_regs).ebx)
> +# define SYSCALL_RET_SET(_regs, _val) ((_regs).eax = (_val))
> +#elif defined(__aarch64__)
> +# define HAVE_ARG0		1
> +# define USE_REGSET		1
> +# define ARCH_REGS		struct user_pt_regs
> +# define SYSCALL_ARG0(_regs)	((_regs).regs[0])
> +# define SYSCALL_RET_SET(_regs, _val) ((_regs).regs[0] = (_val))

[Severity: Medium]
This isn't a bug, but tools/testing/selftests/seccomp/seccomp_bpf.c
already defines syscall_arg(), ARCH_REGS, SYSCALL_NUM, SYSCALL_RET,
SYSCALL_RET_SET, get_syscall(), change_syscall(), and FIXTURE(TRACE_syscall)
with ptrace and seccomp variants across more than a dozen architectures.

Could these test cases be added to seccomp_bpf.c (or share its register
and ptrace helpers) instead of introducing a separate seccomp_ptrace_args
binary that duplicates syscall_arg0, ARCH_REGS, SYSCALL_RET_SET, and
get_regs()/set_regs() for four architectures?

[ ... ]
> +TEST(syscall_exit_stop_preserves_arg_for_restart)
> +{
> +	int pipefd[2];
> +	struct sigaction sa = {
> +		.sa_handler = sighandler,
> +		.sa_flags = SA_RESTART,
> +	};
> +	pid_t tracee;
> +	long last_nr = -1;
> +	int status, poked = 0;
> +
> +	ASSERT_EQ(0, pipe(pipefd));
> +
> +	tracee = fork();
> +	ASSERT_GE(tracee, 0);
> +	if (tracee == 0) {
> +		close(pipefd[0]);
> +		sigaction(SIGUSR1, &sa, NULL);
> +		ptrace(PTRACE_TRACEME, 0, NULL, NULL);
> +		raise(SIGSTOP);
> +		_exit(write(pipefd[1], "x", 1) == 1 ? 0 : 1);
> +	}
> +	close(pipefd[1]);
> +
> +	attach(_metadata, tracee, PTRACE_O_TRACESYSGOOD);
> +	ASSERT_EQ(0, ptrace(PTRACE_SYSCALL, tracee, NULL, NULL));
> +
> +	for (;;) {
> +		struct ptrace_syscall_info info;
> +		long len;
> +
> +		ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
> +			kill(tracee, SIGKILL);
> +		}
> +		if (WIFEXITED(status))
> +			break;
> +		ASSERT_FALSE(WIFSIGNALED(status));
> +
> +		len = get_syscall_info(tracee, &info);
> +		if (len < 0) {
> +			kill(tracee, SIGKILL);
> +			SKIP(return, "PTRACE_GET_SYSCALL_INFO unsupported");
> +		}
> +		if (info.op == PTRACE_SYSCALL_INFO_ENTRY)
> +			last_nr = info.entry.nr;
> +		if (!poked && info.op == PTRACE_SYSCALL_INFO_EXIT &&
> +		    last_nr == __NR_write) {
> +			ARCH_REGS regs;
> +
> +			ASSERT_EQ(0, get_regs(tracee, &regs));
> +			SYSCALL_RET_SET(regs, -ERESTARTSYS);
> +			ASSERT_EQ(0, set_regs(tracee, &regs));
> +			poked = 1;
> +			/* resume into the restart path */
> +			ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL,
> +					    SIGUSR1));
> +			continue;
> +		}
> +		ASSERT_EQ(0, ptrace(PTRACE_SYSCALL, tracee, NULL, NULL));
> +	}

[Severity: Medium]
Does SIGUSR1 actually get delivered to sighandler() in
syscall_exit_stop_preserves_arg_for_restart?

When ptrace(PTRACE_CONT, tracee, NULL, SIGUSR1) is called at the
PTRACE_SYSCALL_INFO_EXIT stop, ptrace_report_syscall() queues SIGUSR1 via
send_sig(). As the tracee exits the syscall into get_signal(),
ptrace_signal() stops the tracee in a ptrace signal-delivery stop for
SIGUSR1.

On the next iteration of the loop, get_syscall_info() returns
info.op == PTRACE_SYSCALL_INFO_NONE and poked is already 1, so the loop
calls:

    ptrace(PTRACE_SYSCALL, tracee, NULL, NULL);

Passing NULL (0) as the signal at the signal-delivery stop cancels
SIGUSR1 in ptrace_signal(), so get_signal() returns false and sighandler()
and the SA_RESTART signal-delivery path are bypassed. Should the
signal-delivery stop forward WSTOPSIG(status) when resuming the tracee?

> +
> +	close(pipefd[0]);
> +	EXPECT_EQ(1, poked);
> +	EXPECT_EQ(0, WEXITSTATUS(status));
> +}
> +
> +#ifdef HAVE_ORIG_ARG0
> +
> +/*
> + * The tracer rewrites r0 while the tracee is stopped on a signal delivered
> + * outside of a syscall, where ARM_ORIG_r0 holds the ~0 "no syscall" marker.
> + * The marker must not be replaced by the value the tracer wrote.
> + */
> +TEST(non_syscall_stop_keeps_orig_arg)
> +{
> +	sig_atomic_t *got_signal = mmap(NULL, sizeof(*got_signal),
> +					PROT_READ | PROT_WRITE,
> +					MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> +	struct sigaction sa = {
> +		.sa_handler = sighandler,
> +	};
> +	pid_t tracee;
> +	int status;
> +	ARCH_REGS regs;
> +	unsigned long before;
> +
> +	ASSERT_TRUE(got_signal != MAP_FAILED);
> +	ASSERT_EQ(0, sigaction(SIGUSR1, &sa, NULL));
> +
> +	tracee = fork();
> +	ASSERT_GE(tracee, 0);
> +	if (tracee == 0) {
> +		ptrace(PTRACE_TRACEME, 0, NULL, NULL);
> +		raise(SIGSTOP);
> +		/*
> +		 * Spin in user mode so that the signal is not delivered on
> +		 * the return path of a syscall.
> +		 */
> +		while (!__atomic_load_n(got_signal, __ATOMIC_RELAXED))
> +			;
> +		_exit(0);
> +	}
> +
> +	__atomic_store_n(got_signal, 0, __ATOMIC_RELAXED);
> +	attach(_metadata, tracee, 0);
> +	ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL, NULL));
> +	usleep(200000);
> +	ASSERT_EQ(0, kill(tracee, SIGUSR1));
> +
> +	ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
> +		kill(tracee, SIGKILL);
> +	}
> +	ASSERT_TRUE(WIFSTOPPED(status));
> +	ASSERT_EQ(SIGUSR1, WSTOPSIG(status));
> +
> +	ASSERT_EQ(0, get_regs(tracee, &regs));
> +	before = SYSCALL_ARG0_ORIG(regs);
> +	SYSCALL_ARG0(regs) = 0xdeadbeefUL;
> +	ASSERT_EQ(0, set_regs(tracee, &regs));
> +	ASSERT_EQ(0, get_regs(tracee, &regs));
> +
> +	if (before == 0xffffffffUL)
> +		EXPECT_EQ(before, (unsigned long)SYSCALL_ARG0_ORIG(regs));
> +
> +	__atomic_store_n(got_signal, 1, __ATOMIC_RELAXED);
> +	ASSERT_EQ(0, ptrace(PTRACE_CONT, tracee, NULL, SIGUSR1));
> +	ASSERT_EQ(tracee, wait_for_stop(tracee, &status, 10000)) {
> +		kill(tracee, SIGKILL);
> +	}
> +}

[Severity: Medium]
Can non_syscall_stop_keeps_orig_arg silently skip its assertion or fault
in the tracee when resuming?

First, after ptrace(PTRACE_CONT, tracee, NULL, NULL), the parent waits
with an unsynchronized usleep(200000) before sending SIGUSR1. If the
tracee has not yet returned from raise(SIGSTOP) (tgkill) and taken a
user-mode exception under load or slow emulation, before
(SYSCALL_ARG0_ORIG(regs)) will still hold the tgkill argument rather than
0xffffffffUL. Because EXPECT_EQ is guarded by:

    if (before == 0xffffffffUL)
        EXPECT_EQ(before, (unsigned long)SYSCALL_ARG0_ORIG(regs));

the test will silently skip its only assertion and pass. Since got_signal
is already in a MAP_SHARED page, could the tracee write a ready state in
user mode after raise(SIGSTOP) returns so the test can wait for it and
unconditionally assert that before == 0xffffffffUL?

Second, while the tracee is stopped in the middle of the C spin loop:

    while (!__atomic_load_n(got_signal, __ATOMIC_RELAXED))
        ;

the tracer sets SYSCALL_ARG0(regs) (ARM_r0) to 0xdeadbeefUL and resumes
the tracee with SIGUSR1 without restoring r0. Because r0 is a
caller-saved general-purpose register that the compiler may use to hold
the got_signal pointer across the loop, corrupting r0 to 0xdeadbeefUL can
cause the tracee to fault with SIGSEGV upon returning from sighandler().
The final wait_for_stop() also does not check WIFEXITED(status) or
WEXITSTATUS(status) == 0, so a crash in the tracee goes unnoticed.

Finally, got_signal is not unmapped with munmap(), and the earlier SKIP()
paths in syscall_entry_stop_sees_rewritten_arg and
syscall_exit_stop_preserves_arg_for_restart return after
kill(tracee, SIGKILL) without reaping tracee via waitpid() or closing
pipefd[0].

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030063630.1218302-1-ruanjinjie@huawei.com?part=2