On Mon, Jan 13, 2025 at 06:34:44PM +0100, Christophe Leroy wrote:
> Le 13/01/2025 à 18:10, Dmitry V. Levin a écrit :
> > Bring syscall_set_return_value() in sync with syscall_get_error(),
> > and let upcoming ptrace/set_syscall_info selftest pass on powerpc.
> >
> > This reverts commit 1b1a3702a65c ("powerpc: Don't negate error in
> > syscall_set_return_value()").
>
> There is a clear detailed explanation in that commit of why it needs to
> be done.
>
> If you think that commit is wrong you have to explain why with at least
> the same level of details.
I'm sorry, I'm not by any means a powerpc expert to explain why that
commit was added in the first place, I wish Michael would be able to do it
himself. All I can say is that for some mysterious reason current
syscall_set_return_value() implementation assumes that in case of an error
regs->gpr[3] has to be negative, while, according to well-tested
syscall_get_error(), it has to be positive.
This is very visible with PTRACE_SET_SYSCALL_INFO that exposes
syscall_set_return_value() to userspace, and, in particular, with the
architecture-agnostic ptrace/set_syscall_info selftest added later in the
series.
> > diff --git a/arch/powerpc/include/asm/syscall.h
> > b/arch/powerpc/include/asm/syscall.h
> > index 3dd36c5e334a..422d7735ace6 100644
> > --- a/arch/powerpc/include/asm/syscall.h
> > +++ b/arch/powerpc/include/asm/syscall.h
> > @@ -82,7 +82,11 @@ static inline void syscall_set_return_value(struct
> > task_struct *task,
> > */
> > if (error) {
> > regs->ccr |= 0x10000000L;
> > - regs->gpr[3] = error;
> > + /*
> > + * In case of an error regs->gpr[3] contains
> > + * a positive ERRORCODE.
> > + */
> > + regs->gpr[3] = -error;
> > } else {
> > regs->ccr &= ~0x10000000L;
> > regs->gpr[3] = val;
--
ldv
