linux-kernel - Re: [PATCH v2 1/7] powerpc: properly negate error in syscall_set_return

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACGkJduSVLThnZkVFc0aJWjwD5AdBPmXLvZsqy2UxEVRrLm7EA@mail.gmail.com>
Date: Thu, 23 Jan 2025 20:11:44 +0100
From: Eugene Syromyatnikov <evgsyr@...il.com>
To: "Dmitry V. Levin" <ldv@...ace.io>
Cc: Christophe Leroy <christophe.leroy@...roup.eu>, Alexey Gladkov <legion@...nel.org>, 
	Oleg Nesterov <oleg@...hat.com>, Michael Ellerman <mpe@...erman.id.au>, Mike Frysinger <vapier@...too.org>, 
	Renzo Davoli <renzo@...unibo.it>, Davide Berardi <berardi.dav@...il.com>, strace-devel@...ts.strace.io, 
	Madhavan Srinivasan <maddy@...ux.ibm.com>, Nicholas Piggin <npiggin@...il.com>, 
	Naveen N Rao <naveen@...nel.org>, linuxppc-dev@...ts.ozlabs.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/7] powerpc: properly negate error in syscall_set_return_value()

On Thu, Jan 23, 2025 at 7:28 PM Dmitry V. Levin <ldv@...ace.io> 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@...il.com
xmpp:esyr@...ber.{ru|org}