linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: "Dmitry V. Levin" <ldv@strace.io>
To: Michael Ellerman <mpe@ellerman.id.au>,
	Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Alexey Gladkov <legion@kernel.org>,
	Eugene Syromyatnikov <evgsyr@gmail.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Madhavan Srinivasan <maddy@linux.ibm.com>,
	Nicholas Piggin <npiggin@gmail.com>,
	Naveen N Rao <naveen@kernel.org>,
	linuxppc-dev@lists.ozlabs.org, strace-devel@lists.strace.io,
	linux-kernel@vger.kernel.org
Subject: [PATCH 2/2] powerpc: fix inconsistencies in syscall error return handling
Date: Mon, 27 Jan 2025 20:14:24 +0200	[thread overview]
Message-ID: <20250127181424.GB1373@strace.io> (raw)
In-Reply-To: <20250127181322.GA1373@strace.io>

Since the introduction of SECCOMP_RET_TRACE support, the kernel supports
simultaneously both the generic kernel -ERRORCODE return value ABI and
the powerpc sc syscall return ABI for PTRACE_EVENT_SECCOMP tracers.
This change is an attempt to address the code inconsistencies in syscall
error return handling that were introduced as a side effect of the dual
ABI support.

Signed-off-by: Dmitry V. Levin <ldv@strace.io>
---
 arch/powerpc/kernel/ptrace/ptrace.c | 23 ++++++++++++++++++++---
 arch/powerpc/kernel/signal.c        | 11 +++--------
 arch/powerpc/kernel/syscall.c       |  6 +++---
 3 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
index 727ed4a14545..3778775bf6ba 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -207,7 +207,7 @@ static int do_seccomp(struct pt_regs *regs)
 	 * syscall parameter. This is different to the ptrace ABI where
 	 * both r3 and orig_gpr3 contain the first syscall parameter.
 	 */
-	regs->gpr[3] = -ENOSYS;
+	syscall_set_return_value(current, regs, -ENOSYS, 0);
 
 	/*
 	 * We use the __ version here because we have already checked
@@ -215,8 +215,18 @@ static int do_seccomp(struct pt_regs *regs)
 	 * have already loaded -ENOSYS into r3, or seccomp has put
 	 * something else in r3 (via SECCOMP_RET_ERRNO/TRACE).
 	 */
-	if (__secure_computing(NULL))
+	if (__secure_computing(NULL)) {
+
+		/*
+		 * Traditionally, both the generic kernel -ERRORCODE return
+		 * value ABI and the powerpc sc syscall return ABI is
+		 * supported.  For consistency, if the former is detected,
+		 * convert it to the latter.
+		 */
+		if (!trap_is_scv(regs) && IS_ERR_VALUE(regs->gpr[3]))
+			syscall_set_return_value(current, regs, regs->gpr[3], 0);
 		return -1;
+	}
 
 	/*
 	 * The syscall was allowed by seccomp, restore the register
@@ -226,6 +236,13 @@ static int do_seccomp(struct pt_regs *regs)
 	 * allow the syscall to proceed.
 	 */
 	regs->gpr[3] = regs->orig_gpr3;
+	if (!trap_is_scv(regs)) {
+		/*
+		 * Clear SO bit that was set in this function earlier by
+		 * syscall_set_return_value.
+		 */
+		regs->ccr &= ~0x10000000L;
+	}
 
 	return 0;
 }
@@ -315,7 +332,7 @@ long do_syscall_trace_enter(struct pt_regs *regs)
 	 * If we are aborting explicitly, or if the syscall number is
 	 * now invalid, set the return value to -ENOSYS.
 	 */
-	regs->gpr[3] = -ENOSYS;
+	syscall_set_return_value(current, regs, -ENOSYS, 0);
 	return -1;
 }
 
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index aa17e62f3754..1a38d6bcaed6 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -19,6 +19,7 @@
 #include <asm/unistd.h>
 #include <asm/debug.h>
 #include <asm/tm.h>
+#include <asm/syscall.h>
 
 #include "signal.h"
 
@@ -229,14 +230,8 @@ static void check_syscall_restart(struct pt_regs *regs, struct k_sigaction *ka,
 		regs_add_return_ip(regs, -4);
 		regs->result = 0;
 	} else {
-		if (trap_is_scv(regs)) {
-			regs->result = -EINTR;
-			regs->gpr[3] = -EINTR;
-		} else {
-			regs->result = -EINTR;
-			regs->gpr[3] = EINTR;
-			regs->ccr |= 0x10000000;
-		}
+		regs->result = -EINTR;
+		syscall_set_return_value(current, regs, -EINTR, 0);
 	}
 }
 
diff --git a/arch/powerpc/kernel/syscall.c b/arch/powerpc/kernel/syscall.c
index be159ad4b77b..2fe47191e509 100644
--- a/arch/powerpc/kernel/syscall.c
+++ b/arch/powerpc/kernel/syscall.c
@@ -122,7 +122,7 @@ notrace long system_call_exception(struct pt_regs *regs, unsigned long r0)
 		if (unlikely(trap_is_unsupported_scv(regs))) {
 			/* Unsupported scv vector */
 			_exception(SIGILL, regs, ILL_ILLOPC, regs->nip);
-			return regs->gpr[3];
+			return regs_return_value(regs);
 		}
 		/*
 		 * We use the return value of do_syscall_trace_enter() as the
@@ -133,13 +133,13 @@ notrace long system_call_exception(struct pt_regs *regs, unsigned long r0)
 		 */
 		r0 = do_syscall_trace_enter(regs);
 		if (unlikely(r0 >= NR_syscalls))
-			return regs->gpr[3];
+			return regs_return_value(regs);
 
 	} else if (unlikely(r0 >= NR_syscalls)) {
 		if (unlikely(trap_is_unsupported_scv(regs))) {
 			/* Unsupported scv vector */
 			_exception(SIGILL, regs, ILL_ILLOPC, regs->nip);
-			return regs->gpr[3];
+			return regs_return_value(regs);
 		}
 		return -ENOSYS;
 	}
-- 
ldv


  reply	other threads:[~2025-01-27 18:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-27 18:13 [PATCH 1/2] powerpc: properly negate error in syscall_set_return_value() in sc case Dmitry V. Levin
2025-01-27 18:14 ` Dmitry V. Levin [this message]
2025-01-28 18:01   ` [PATCH 2/2] powerpc: fix inconsistencies in syscall error return handling Christophe Leroy
2025-01-28 21:39     ` Dmitry V. Levin
2025-01-28 14:59 ` [PATCH 1/2] powerpc: properly negate error in syscall_set_return_value() in sc case Christophe Leroy
2025-01-28 15:52   ` Dmitry V. Levin
2025-01-28 16:00     ` Christophe Leroy
2025-01-28 16:10       ` Dmitry V. Levin
2025-01-28 15:54   ` Eugene Syromyatnikov
2025-06-02 11:36 ` Dmitry V. Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250127181424.GB1373@strace.io \
    --to=ldv@strace.io \
    --cc=christophe.leroy@csgroup.eu \
    --cc=evgsyr@gmail.com \
    --cc=legion@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=mpe@ellerman.id.au \
    --cc=naveen@kernel.org \
    --cc=npiggin@gmail.com \
    --cc=oleg@redhat.com \
    --cc=strace-devel@lists.strace.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).