Re: [PATCH v5 0/2] PTRACE_SET_SYSCALL_INFO: add support for seccomp syscall skipping

[Renzo Davoli]

Hi Michal,

Thank you for pointing out the PowerPC issue. I spent some time tracing the
syscall entry/exit path on a current kernel (git 0e35b9b6ec0ff + my
syscall_skip patch) to understand where the return value is lost.

The path I observed is the following:

ptrace_set_syscall_info()
        syscall_set_nr(..., -1);
        syscall_set_return_value(..., 42);

system_call_exception()
        r0 = syscall_enter_from_user_mode(...);
        if (r0 >= NR_syscalls)
                return -ENOSYS;
interrupt_64.S
        bl system_call_exception
        bl syscall_exit_prepare

syscall_exit_prepare(r3, regs, ...)
        regs->gpr[3] = r3;

Initially I thought the return value from system_call_exception() was
ignored, but the assembly passes the PPC64 ABI return register directly as the
first argument to syscall_exit_prepare(). Consequently, the -ENOSYS
returned by system_call_exception() becomes the value written back into
regs->gpr[3].

To verify this, I added a few temporary pr_info() statements.

Immediately after syscall_set_return_value():
ptrace: regs->gpr[3] = 42

and at the beginning of syscall_exit_prepare():
syscall_exit_prepare:
        r3 = -38
        regs->gpr[3] = 42

where -38 is -ENOSYS.

syscall_exit_prepare() then executes
regs->gpr[3] = r3;

so the value installed by syscall_set_return_value() is overwritten before
returning to userspace.

This reproduces exactly the behavior you described: on PowerPC, a return value
preset before the syscall is skipped is not preserved through the architecture
exit path.

This also seems inconsistent with the generic contract documented in
include/linux/entry-common.h, namely that if a skipped syscall already has a
return value installed via syscall_set_return_value(), that value should be
returned; otherwise the default result is -ENOSYS. The current PowerPC exit
path always installs the default value.

Given these observations, it seems that the ptrace patch is exposing an
existing architectural issue rather than introducing a new one.

Regarding possible solutions, I currently see two possible directions.

*  Introduce a pt_regs flag indicating that the final userspace return value has
already been installed (as you suggested), so that syscall_exit_prepare() does
not overwrite it.
>From the PowerPC code path I investigated, this appears to be sufficient. The
value installed by syscall_set_return_value() survives in regs->gpr[3] until
syscall_exit_prepare(), where it is unconditionally replaced by the default
-ENOSYS return value. In other words, the issue appears to be confined to the
architecture-specific exit path.

* Extend the generic syscall API (for example by introducing new helpers in
asm/syscall.h or changing the semantics of the existing helpers) so that
architectures can explicitly represent a final userspace return value.

My impression is that Sven's proposal addresses a different problem, namely
architectures where the live register state cannot safely hold the pending
return value during syscall entry because it overlaps with the syscall number.
On PowerPC, however, the value installed by syscall_set_return_value() is still
present in regs->gpr[3] when syscall_exit_prepare() is entered, so the issue
seems to be deciding whether the architecture should preserve that value or
replace it with the default -ENOSYS.

For this reason I am currently inclined towards the first approach for PowerPC,
as it appears to be the minimal architecture-specific change required to
restore the documented API contract.

Does this match your understanding of the PowerPC exit path? If so, would you
consider the flag approach the preferred direction for PowerPC?

Thanks,
renzo