From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from vmicros1.altlinux.org (vmicros1.altlinux.org [194.107.17.57]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 048561B0F2F for ; Thu, 23 Jan 2025 22:16:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=194.107.17.57 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737670610; cv=none; b=fDzg6aWx3A0Q30dajOScCJcdcUg16hF+3dUkOiKgKLS6NRICuIBE2PYKW43RplPR0rsQO9FtZAzX4Gs7+z4N4L0ByN58Y/xyLcbMg/ivgD8n0n8ZhbITc3ULbMINpyLRkLAwVqa4Qsx5Q98dOIr4GZlgreMosp7yJ5EF/RRTwd8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737670610; c=relaxed/simple; bh=wmPpTXexutskMM4+2vMTLBF994nUjvYcG7NP71dUUEE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=SpNzQh4L6SqQthR9Zfu7SGnUy8SkwNgLkE3V7pM96mOWjeClCpKAWLCTgsNyhOJXQuWOgGh2bqmynSx47s/3/knCigmGpRYuTrBquVy4R+ZYZ7myudI4j/MFHSiY6EWNPgOKdoMcDsxCtV9Vyy94sxe+ZEm10DhC6gQR23bAoXw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=strace.io; spf=pass smtp.mailfrom=altlinux.org; arc=none smtp.client-ip=194.107.17.57 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=strace.io Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=altlinux.org Received: from mua.local.altlinux.org (mua.local.altlinux.org [192.168.1.14]) by vmicros1.altlinux.org (Postfix) with ESMTP id 4AD2072C8CC; Fri, 24 Jan 2025 01:16:46 +0300 (MSK) Received: by mua.local.altlinux.org (Postfix, from userid 508) id 365237CCB3A; Fri, 24 Jan 2025 00:16:46 +0200 (IST) Date: Fri, 24 Jan 2025 00:16:46 +0200 From: "Dmitry V. Levin" To: Eugene Syromyatnikov Cc: Christophe Leroy , Alexey Gladkov , Oleg Nesterov , Michael Ellerman , Mike Frysinger , Renzo Davoli , Davide Berardi , strace-devel@lists.strace.io, Madhavan Srinivasan , Nicholas Piggin , Naveen N Rao , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/7] powerpc: properly negate error in syscall_set_return_value() Message-ID: <20250123221646.GA22814@strace.io> References: <20250113171054.GA589@strace.io> <6558110c-c2cb-4aa3-9472-b3496f71ebb8@csgroup.eu> <20250114170400.GB11820@strace.io> <20250123182815.GA20994@strace.io> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Thu, Jan 23, 2025 at 08:11:44PM +0100, Eugene Syromyatnikov wrote: > On Thu, Jan 23, 2025 at 7:28 PM Dmitry V. Levin 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 The semantics of r3 on syscall return is correct, thanks to syscall_exit_prepare() that performs necessary manipulations with gpr[3]. What's wrong on powerpc in !trap_is_scv case is that its current implementation of syscall_set_return_value() follows a different semantics, making it unusable on syscall return. While syscall_set_return_value() was used only on entering syscall via do_seccomp(), it was not a problem yet. It became a problem when we started to use it on syscall return, in the same state when its sibling syscall_get_error() is used. Note that among all the architectures in the kernel tree powerpc in !trap_is_scv case is the only one that has this problem. My patch is intended to address this without breaking anything else. -- ldv