Re: [PATCH v2 1/7] powerpc: properly negate error in syscall_set_return_value()

[Eugene Syromyatnikov]

On Thu, Jan 23, 2025 at 7:28 PM Dmitry V. Levin <ldv@xxxxxxxxx> wrote:
> Indeed, there is an inconsistency in !trap_is_scv case.
>
> In some places such as syscall_get_error() and regs_return_value() the
> semantics is as I described earlier: gpr[3] contains a positive ERRORCODE
> and ccr has 0x10000000 flag set.  This semantics is a part of the ABI and
> therefore cannot be changed.
>
> In some other places like do_seccomp() and do_syscall_trace_enter() the
> semantics is similar to the trap_is_scv case: gpr[3] contains a negative
> ERRORCODE and ccr is unchanged.  In addition, system_call_exception()
> returns the system call function return value when it is executed, and
> gpr[3] otherwise.  The value returned by system_call_exception() is passed
> on to syscall_exit_prepare() which performs the conversion you mentioned.
>
> What's remarkable is that in those places that are a part of the ABI the
> traditional semantics is kept, while in other places the implementation
> follows the trap_is_scv-like semantics, while traditional semantics is
> also supported there.
>
> The only case where I see some intersection is do_seccomp() where the
> tracer would be able to see -ENOSYS in gpr[3].  However, the seccomp stop
> is not the place where the tracer *reads* the system call exit status,
> so whatever was written in gpr[3] before __secure_computing() is not
> really relevant, consequently, selftests/seccomp/seccomp_bpf passes with
> this patch applied as well as without it.
>
> After looking at system_call_exception() I doubt this inconsistency can be
> easily avoided, so I don't see how this patch could be enhanced further,
> and what else could I do with the patch besides dropping it and letting
> !trap_is_scv case be unsupported by PTRACE_SET_SYSCALL_INFO API, which
> would be unfortunate.

The semantics of r3 on syscall return (including the negatedness of
the errno value) is documented in [1] (at least for the 64-bit case,
but I conjecture the 32-bit one is the same, sans the lack of the v2
ABI and scv there), so I would suggest to consider any deviation from
that a kernel programming error to be fixed.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arch/powerpc/syscall64-abi.rst?id=v6.13#n30

-- 
Eugene Syromyatnikov
mailto:evgsyr@xxxxxxxxx
xmpp:esyr@jabber.{ru|org}