[PATCH v4 0/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API

Dmitry V. Levin posted 7 patches 1 year ago
There is a newer version of this series
arch/arc/include/asm/syscall.h                |  25 +
arch/arm/include/asm/syscall.h                |  37 ++
arch/arm64/include/asm/syscall.h              |  29 +
arch/csky/include/asm/syscall.h               |  13 +
arch/hexagon/include/asm/syscall.h            |  21 +
arch/loongarch/include/asm/syscall.h          |  15 +
arch/m68k/include/asm/syscall.h               |   7 +
arch/microblaze/include/asm/syscall.h         |   7 +
arch/mips/include/asm/syscall.h               |  70 ++-
arch/nios2/include/asm/syscall.h              |  16 +
arch/openrisc/include/asm/syscall.h           |  13 +
arch/parisc/include/asm/syscall.h             |  19 +
arch/powerpc/include/asm/syscall.h            |  20 +
arch/riscv/include/asm/syscall.h              |  16 +
arch/s390/include/asm/syscall.h               |  21 +
arch/sh/include/asm/syscall_32.h              |  24 +
arch/sparc/include/asm/syscall.h              |  22 +
arch/um/include/asm/syscall-generic.h         |  19 +
arch/x86/include/asm/syscall.h                |  43 ++
arch/xtensa/include/asm/syscall.h             |  18 +
include/asm-generic/syscall.h                 |  30 +
include/uapi/linux/ptrace.h                   |   7 +-
kernel/ptrace.c                               | 179 +++++-
tools/testing/selftests/ptrace/Makefile       |   2 +-
.../selftests/ptrace/set_syscall_info.c       | 514 ++++++++++++++++++
25 files changed, 1140 insertions(+), 47 deletions(-)
create mode 100644 tools/testing/selftests/ptrace/set_syscall_info.c
[PATCH v4 0/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API
Posted by Dmitry V. Levin 1 year ago
PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements
PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of
system calls the tracee is blocked in.

This API allows ptracers to obtain and modify system call details
in a straightforward and architecture-agnostic way.

Current implementation supports changing only those bits of system call
information that are used by strace, namely, syscall number, syscall
arguments, and syscall return value.

Support of changing additional details returned by PTRACE_GET_SYSCALL_INFO,
such as instruction pointer and stack pointer, could be added later if
needed, by using struct ptrace_syscall_info.flags to specify the additional
details that should be set.  Currently, "flags" and "reserved" fields of
struct ptrace_syscall_info must be initialized with zeroes; "arch",
"instruction_pointer", and "stack_pointer" fields are ignored.

PTRACE_SET_SYSCALL_INFO currently supports only PTRACE_SYSCALL_INFO_ENTRY,
PTRACE_SYSCALL_INFO_EXIT, and PTRACE_SYSCALL_INFO_SECCOMP operations.
Other operations could be added later if needed.

Ideally, PTRACE_SET_SYSCALL_INFO should have been introduced along with
PTRACE_GET_SYSCALL_INFO, but it didn't happen.  The last straw that
convinced me to implement PTRACE_SET_SYSCALL_INFO was apparent failure
to provide an API of changing the first system call argument on riscv
architecture [1].

ptrace(2) man page:

long ptrace(enum __ptrace_request request, pid_t pid, void *addr, void *data);
...
PTRACE_SET_SYSCALL_INFO
       Modify information about the system call that caused the stop.
       The "data" argument is a pointer to struct ptrace_syscall_info
       that specifies the system call information to be set.
       The "addr" argument should be set to sizeof(struct ptrace_syscall_info)).

[1] https://lore.kernel.org/all/59505464-c84a-403d-972f-d4b2055eeaac@gmail.com/

Notes:
    v4:
    * Split out syscall_set_return_value() for hexagon into a separate patch
    * s390: Change the style of syscall_set_arguments() implementation as
      requested
    * Add more Reviewed-by
    * v3: https://lore.kernel.org/all/20250128091445.GA8257@strace.io/

    v3:
    * powerpc: Submit syscall_set_return_value() fix for "sc" case separately
    * mips: Do not introduce erroneous argument truncation on mips n32,
      add a detailed description to the commit message of the
      mips_get_syscall_arg() change
    * ptrace: Add explicit padding to the end of struct ptrace_syscall_info,
      simplify obtaining of user ptrace_syscall_info,
      do not introduce PTRACE_SYSCALL_INFO_SIZE_VER0
    * ptrace: Change the return type of ptrace_set_syscall_info_* functions
      from "unsigned long" to "int"
    * ptrace: Add -ERANGE check to ptrace_set_syscall_info_exit(),
      add comments to -ERANGE checks
    * ptrace: Update comments about supported syscall stops
    * selftests: Extend set_syscall_info test, fix for mips n32
    * Add Tested-by and Reviewed-by

    v2:
    * Add patch to fix syscall_set_return_value() on powerpc
    * Add patch to fix mips_get_syscall_arg() on mips
    * Add syscall_set_return_value() implementation on hexagon
    * Add syscall_set_return_value() invocation to syscall_set_nr()
      on arm and arm64.
    * Fix syscall_set_nr() and mips_set_syscall_arg() on mips
    * Add a comment to syscall_set_nr() on arc, powerpc, s390, sh,
      and sparc
    * Remove redundant ptrace_syscall_info.op assignments in
      ptrace_get_syscall_info_*
    * Minor style tweaks in ptrace_get_syscall_info_op()
    * Remove syscall_set_return_value() invocation from
      ptrace_set_syscall_info_entry()
    * Skip syscall_set_arguments() invocation in case of syscall number -1
      in ptrace_set_syscall_info_entry() 
    * Split ptrace_syscall_info.reserved into ptrace_syscall_info.reserved
      and ptrace_syscall_info.flags
    * Use __kernel_ulong_t instead of unsigned long in set_syscall_info test

    v1:

Dmitry V. Levin (7):
  mips: fix mips_get_syscall_arg() for o32
  hexagon: add syscall_set_return_value()
  syscall.h: add syscall_set_arguments()
  syscall.h: introduce syscall_set_nr()
  ptrace_get_syscall_info: factor out ptrace_get_syscall_info_op
  ptrace: introduce PTRACE_SET_SYSCALL_INFO request
  selftests/ptrace: add a test case for PTRACE_SET_SYSCALL_INFO

 arch/arc/include/asm/syscall.h                |  25 +
 arch/arm/include/asm/syscall.h                |  37 ++
 arch/arm64/include/asm/syscall.h              |  29 +
 arch/csky/include/asm/syscall.h               |  13 +
 arch/hexagon/include/asm/syscall.h            |  21 +
 arch/loongarch/include/asm/syscall.h          |  15 +
 arch/m68k/include/asm/syscall.h               |   7 +
 arch/microblaze/include/asm/syscall.h         |   7 +
 arch/mips/include/asm/syscall.h               |  70 ++-
 arch/nios2/include/asm/syscall.h              |  16 +
 arch/openrisc/include/asm/syscall.h           |  13 +
 arch/parisc/include/asm/syscall.h             |  19 +
 arch/powerpc/include/asm/syscall.h            |  20 +
 arch/riscv/include/asm/syscall.h              |  16 +
 arch/s390/include/asm/syscall.h               |  21 +
 arch/sh/include/asm/syscall_32.h              |  24 +
 arch/sparc/include/asm/syscall.h              |  22 +
 arch/um/include/asm/syscall-generic.h         |  19 +
 arch/x86/include/asm/syscall.h                |  43 ++
 arch/xtensa/include/asm/syscall.h             |  18 +
 include/asm-generic/syscall.h                 |  30 +
 include/uapi/linux/ptrace.h                   |   7 +-
 kernel/ptrace.c                               | 179 +++++-
 tools/testing/selftests/ptrace/Makefile       |   2 +-
 .../selftests/ptrace/set_syscall_info.c       | 514 ++++++++++++++++++
 25 files changed, 1140 insertions(+), 47 deletions(-)
 create mode 100644 tools/testing/selftests/ptrace/set_syscall_info.c

-- 
ldv
Re: [PATCH v4 0/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API
Posted by Alexander Gordeev 1 year ago
On Mon, Feb 03, 2025 at 08:58:49AM +0200, Dmitry V. Levin wrote:

Hi Dmitry,

> PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements
> PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of
> system calls the tracee is blocked in.
...

FWIW, I am getting these on s390:

# ./tools/testing/selftests/ptrace/set_syscall_info 
TAP version 13
1..1
# Starting 1 tests from 1 test cases.
#  RUN           global.set_syscall_info ...
# set_syscall_info.c:87:set_syscall_info:Expected exp_entry->nr (-1) == info->entry.nr (65535)
# set_syscall_info.c:88:set_syscall_info:wait #3: PTRACE_GET_SYSCALL_INFO #2: syscall nr mismatch
# set_syscall_info: Test terminated by assertion
#          FAIL  global.set_syscall_info
not ok 1 global.set_syscall_info
# FAILED: 0 / 1 tests passed.
# Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0

I remember one of the earlier versions (v1 or v2) was working for me.

Thanks!
Re: [PATCH v4 0/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API
Posted by Dmitry V. Levin 1 year ago
On Mon, Feb 03, 2025 at 10:29:37AM +0100, Alexander Gordeev wrote:
> On Mon, Feb 03, 2025 at 08:58:49AM +0200, Dmitry V. Levin wrote:
> 
> Hi Dmitry,
> 
> > PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements
> > PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of
> > system calls the tracee is blocked in.
> ...
> 
> FWIW, I am getting these on s390:
> 
> # ./tools/testing/selftests/ptrace/set_syscall_info 
> TAP version 13
> 1..1
> # Starting 1 tests from 1 test cases.
> #  RUN           global.set_syscall_info ...
> # set_syscall_info.c:87:set_syscall_info:Expected exp_entry->nr (-1) == info->entry.nr (65535)
> # set_syscall_info.c:88:set_syscall_info:wait #3: PTRACE_GET_SYSCALL_INFO #2: syscall nr mismatch
> # set_syscall_info: Test terminated by assertion
> #          FAIL  global.set_syscall_info
> not ok 1 global.set_syscall_info
> # FAILED: 0 / 1 tests passed.
> # Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0
> 
> I remember one of the earlier versions (v1 or v2) was working for me.
> 
> Thanks!

In v3, this test was extended to check whether PTRACE_GET_SYSCALL_INFO
called immediately after PTRACE_SET_SYSCALL_INFO returns the same syscall
number, and on s390 it apparently doesn't, thanks to its implementation
of syscall_get_nr() that returns 0xffff in this case.

To workaround this, we could either change syscall_get_nr() to return -1
in this case, or add an #ifdef __s390x__ exception to the test.

What would you prefer?


-- 
ldv
Re: [PATCH v4 0/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API
Posted by Dmitry V. Levin 1 year ago
On Mon, Feb 03, 2025 at 12:35:42PM +0200, Dmitry V. Levin wrote:
> On Mon, Feb 03, 2025 at 10:29:37AM +0100, Alexander Gordeev wrote:
> > On Mon, Feb 03, 2025 at 08:58:49AM +0200, Dmitry V. Levin wrote:
> > 
> > Hi Dmitry,
> > 
> > > PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements
> > > PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of
> > > system calls the tracee is blocked in.
> > ...
> > 
> > FWIW, I am getting these on s390:
> > 
> > # ./tools/testing/selftests/ptrace/set_syscall_info 
> > TAP version 13
> > 1..1
> > # Starting 1 tests from 1 test cases.
> > #  RUN           global.set_syscall_info ...
> > # set_syscall_info.c:87:set_syscall_info:Expected exp_entry->nr (-1) == info->entry.nr (65535)
> > # set_syscall_info.c:88:set_syscall_info:wait #3: PTRACE_GET_SYSCALL_INFO #2: syscall nr mismatch
> > # set_syscall_info: Test terminated by assertion
> > #          FAIL  global.set_syscall_info
> > not ok 1 global.set_syscall_info
> > # FAILED: 0 / 1 tests passed.
> > # Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0
> > 
> > I remember one of the earlier versions (v1 or v2) was working for me.
> > 
> > Thanks!
> 
> In v3, this test was extended to check whether PTRACE_GET_SYSCALL_INFO
> called immediately after PTRACE_SET_SYSCALL_INFO returns the same syscall
> number, and on s390 it apparently doesn't, thanks to its implementation
> of syscall_get_nr() that returns 0xffff in this case.
> 
> To workaround this, we could either change syscall_get_nr() to return -1
> in this case, or add an #ifdef __s390x__ exception to the test.
> 
> What would you prefer?

OK, I'm going to apply the following s390 workaround to the test:

diff --git a/tools/testing/selftests/ptrace/set_syscall_info.c b/tools/testing/selftests/ptrace/set_syscall_info.c
index 0ec69401c008..4198248ef874 100644
--- a/tools/testing/selftests/ptrace/set_syscall_info.c
+++ b/tools/testing/selftests/ptrace/set_syscall_info.c
@@ -71,6 +71,11 @@ check_psi_entry(struct __test_metadata *_metadata,
 		const char *text)
 {
 	unsigned int i;
+	int exp_nr = exp_entry->nr;
+#if defined __s390__ || defined __s390x__
+	/* s390 is the only architecture that has 16-bit syscall numbers */
+	exp_nr &= 0xffff;
+#endif
 
 	ASSERT_EQ(PTRACE_SYSCALL_INFO_ENTRY, info->op) {
 		LOG_KILL_TRACEE("%s: entry stop mismatch", text);
@@ -84,7 +89,7 @@ check_psi_entry(struct __test_metadata *_metadata,
 	ASSERT_TRUE(info->stack_pointer) {
 		LOG_KILL_TRACEE("%s: entry stop mismatch", text);
 	}
-	ASSERT_EQ(exp_entry->nr, info->entry.nr) {
+	ASSERT_EQ(exp_nr, info->entry.nr) {
 		LOG_KILL_TRACEE("%s: syscall nr mismatch", text);
 	}
 	for (i = 0; i < ARRAY_SIZE(exp_entry->args); ++i) {

-- 
ldv
Re: [PATCH v4 0/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API
Posted by Sven Schnelle 1 year ago
"Dmitry V. Levin" <ldv@strace.io> writes:

> On Mon, Feb 03, 2025 at 12:35:42PM +0200, Dmitry V. Levin wrote:
>> On Mon, Feb 03, 2025 at 10:29:37AM +0100, Alexander Gordeev wrote:
>> > On Mon, Feb 03, 2025 at 08:58:49AM +0200, Dmitry V. Levin wrote:
>> > 
>> > Hi Dmitry,
>> > 
>> > > PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements
>> > > PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of
>> > > system calls the tracee is blocked in.
>> > ...
>> > 
>> > FWIW, I am getting these on s390:
>> > 
>> > # ./tools/testing/selftests/ptrace/set_syscall_info 
>> > TAP version 13
>> > 1..1
>> > # Starting 1 tests from 1 test cases.
>> > #  RUN           global.set_syscall_info ...
>> > # set_syscall_info.c:87:set_syscall_info:Expected exp_entry->nr (-1) == info->entry.nr (65535)
>> > # set_syscall_info.c:88:set_syscall_info:wait #3: PTRACE_GET_SYSCALL_INFO #2: syscall nr mismatch
>> > # set_syscall_info: Test terminated by assertion
>> > #          FAIL  global.set_syscall_info
>> > not ok 1 global.set_syscall_info
>> > # FAILED: 0 / 1 tests passed.
>> > # Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0
>> > 
>> > I remember one of the earlier versions (v1 or v2) was working for me.
>> > 
>> > Thanks!
>> 
>> In v3, this test was extended to check whether PTRACE_GET_SYSCALL_INFO
>> called immediately after PTRACE_SET_SYSCALL_INFO returns the same syscall
>> number, and on s390 it apparently doesn't, thanks to its implementation
>> of syscall_get_nr() that returns 0xffff in this case.
>> 
>> To workaround this, we could either change syscall_get_nr() to return -1
>> in this case, or add an #ifdef __s390x__ exception to the test.
>> 
>> What would you prefer?
>
> OK, I'm going to apply the following s390 workaround to the test:
>
> diff --git a/tools/testing/selftests/ptrace/set_syscall_info.c b/tools/testing/selftests/ptrace/set_syscall_info.c
> index 0ec69401c008..4198248ef874 100644
> --- a/tools/testing/selftests/ptrace/set_syscall_info.c
> +++ b/tools/testing/selftests/ptrace/set_syscall_info.c
> @@ -71,6 +71,11 @@ check_psi_entry(struct __test_metadata *_metadata,
>  		const char *text)
>  {
>  	unsigned int i;
> +	int exp_nr = exp_entry->nr;
> +#if defined __s390__ || defined __s390x__
> +	/* s390 is the only architecture that has 16-bit syscall numbers */
> +	exp_nr &= 0xffff;
> +#endif
>  
>  	ASSERT_EQ(PTRACE_SYSCALL_INFO_ENTRY, info->op) {
>  		LOG_KILL_TRACEE("%s: entry stop mismatch", text);
> @@ -84,7 +89,7 @@ check_psi_entry(struct __test_metadata *_metadata,
>  	ASSERT_TRUE(info->stack_pointer) {
>  		LOG_KILL_TRACEE("%s: entry stop mismatch", text);
>  	}
> -	ASSERT_EQ(exp_entry->nr, info->entry.nr) {
> +	ASSERT_EQ(exp_nr, info->entry.nr) {
>  		LOG_KILL_TRACEE("%s: syscall nr mismatch", text);
>  	}
>  	for (i = 0; i < ARRAY_SIZE(exp_entry->args); ++i) {

Fine with me. As you already noted only 16 bit of the syscall number is
stored in pt_regs::int_code. A quick hack would be possible to do sign
extensions, so -1 would work. But i think this would be odd, because
positive numbers would still be limited.

So i think the patch you proposed is fine.
Re: [PATCH v4 0/7] ptrace: introduce PTRACE_SET_SYSCALL_INFO API
Posted by Dmitry V. Levin 12 months ago
On Wed, Feb 05, 2025 at 10:06:36AM +0100, Sven Schnelle wrote:
> "Dmitry V. Levin" <ldv@strace.io> writes:
> > On Mon, Feb 03, 2025 at 12:35:42PM +0200, Dmitry V. Levin wrote:
> >> On Mon, Feb 03, 2025 at 10:29:37AM +0100, Alexander Gordeev wrote:
> >> > On Mon, Feb 03, 2025 at 08:58:49AM +0200, Dmitry V. Levin wrote:
> >> > 
> >> > Hi Dmitry,
> >> > 
> >> > > PTRACE_SET_SYSCALL_INFO is a generic ptrace API that complements
> >> > > PTRACE_GET_SYSCALL_INFO by letting the ptracer modify details of
> >> > > system calls the tracee is blocked in.
> >> > ...
> >> > 
> >> > FWIW, I am getting these on s390:
> >> > 
> >> > # ./tools/testing/selftests/ptrace/set_syscall_info 
> >> > TAP version 13
> >> > 1..1
> >> > # Starting 1 tests from 1 test cases.
> >> > #  RUN           global.set_syscall_info ...
> >> > # set_syscall_info.c:87:set_syscall_info:Expected exp_entry->nr (-1) == info->entry.nr (65535)
> >> > # set_syscall_info.c:88:set_syscall_info:wait #3: PTRACE_GET_SYSCALL_INFO #2: syscall nr mismatch
> >> > # set_syscall_info: Test terminated by assertion
> >> > #          FAIL  global.set_syscall_info
> >> > not ok 1 global.set_syscall_info
> >> > # FAILED: 0 / 1 tests passed.
> >> > # Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0
> >> > 
> >> > I remember one of the earlier versions (v1 or v2) was working for me.
> >> > 
> >> > Thanks!
> >> 
> >> In v3, this test was extended to check whether PTRACE_GET_SYSCALL_INFO
> >> called immediately after PTRACE_SET_SYSCALL_INFO returns the same syscall
> >> number, and on s390 it apparently doesn't, thanks to its implementation
> >> of syscall_get_nr() that returns 0xffff in this case.
> >> 
> >> To workaround this, we could either change syscall_get_nr() to return -1
> >> in this case, or add an #ifdef __s390x__ exception to the test.
> >> 
> >> What would you prefer?
> >
> > OK, I'm going to apply the following s390 workaround to the test:
> >
> > diff --git a/tools/testing/selftests/ptrace/set_syscall_info.c b/tools/testing/selftests/ptrace/set_syscall_info.c
> > index 0ec69401c008..4198248ef874 100644
> > --- a/tools/testing/selftests/ptrace/set_syscall_info.c
> > +++ b/tools/testing/selftests/ptrace/set_syscall_info.c
> > @@ -71,6 +71,11 @@ check_psi_entry(struct __test_metadata *_metadata,
> >  		const char *text)
> >  {
> >  	unsigned int i;
> > +	int exp_nr = exp_entry->nr;
> > +#if defined __s390__ || defined __s390x__
> > +	/* s390 is the only architecture that has 16-bit syscall numbers */
> > +	exp_nr &= 0xffff;
> > +#endif
> >  
> >  	ASSERT_EQ(PTRACE_SYSCALL_INFO_ENTRY, info->op) {
> >  		LOG_KILL_TRACEE("%s: entry stop mismatch", text);
> > @@ -84,7 +89,7 @@ check_psi_entry(struct __test_metadata *_metadata,
> >  	ASSERT_TRUE(info->stack_pointer) {
> >  		LOG_KILL_TRACEE("%s: entry stop mismatch", text);
> >  	}
> > -	ASSERT_EQ(exp_entry->nr, info->entry.nr) {
> > +	ASSERT_EQ(exp_nr, info->entry.nr) {
> >  		LOG_KILL_TRACEE("%s: syscall nr mismatch", text);
> >  	}
> >  	for (i = 0; i < ARRAY_SIZE(exp_entry->args); ++i) {
> 
> Fine with me. As you already noted only 16 bit of the syscall number is
> stored in pt_regs::int_code. A quick hack would be possible to do sign
> extensions, so -1 would work. But i think this would be odd, because
> positive numbers would still be limited.
> 
> So i think the patch you proposed is fine.

Thanks, I've applied this workaround in v5:
https://lore.kernel.org/all/20250210113336.GA887@strace.io/

Please consider adding your Reviewed-by.


-- 
ldv