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] [day] [month] [year] [list]

Message-ID: <20250127122606.GA30489@strace.io>
Date: Mon, 27 Jan 2025 14:26:06 +0200
From: "Dmitry V. Levin" <ldv@...ace.io>
To: Christophe Leroy <christophe.leroy@...roup.eu>
Cc: Alexey Gladkov <legion@...nel.org>, Oleg Nesterov <oleg@...hat.com>,
	Michael Ellerman <mpe@...erman.id.au>,
	Eugene Syromyatnikov <evgsyr@...il.com>,
	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 Mon, Jan 27, 2025 at 01:04:27PM +0100, Christophe Leroy wrote:
> 
> 
> Le 27/01/2025 à 12:44, Dmitry V. Levin a écrit :
> > On Mon, Jan 27, 2025 at 12:36:53PM +0100, Christophe Leroy wrote:
> >> Le 27/01/2025 à 12:20, Dmitry V. Levin a écrit :
> >>> On Thu, Jan 23, 2025 at 11:07:21PM +0100, Christophe Leroy wrote:
> >>> [...]
> >>>> To add a bit more to the confusion,
> >>>
> >>> Looks like there is no end to it:
> >>>
> >>> static inline long regs_return_value(struct pt_regs *regs)
> >>> {
> >>>           if (trap_is_scv(regs))
> >>>                   return regs->gpr[3];
> >>>
> >>>           if (is_syscall_success(regs))
> >>>                   return regs->gpr[3];
> >>>           else
> >>>                   return -regs->gpr[3];
> >>> }
> >>>
> >>> static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc)
> >>> {
> >>>           regs->gpr[3] = rc;
> >>> }
> >>>
> >>> This doesn't look consistent, does it?
> >>>
> >>>
> >>
> >> That regs_set_return_value() looks pretty similar to
> >> syscall_get_return_value().
> > 
> > Yes, but here similarities end, and differences begin.
> > 
> >> regs_set_return_value() documentation in asm-generic/syscall.h
> >> explicitely says: This value is meaningless if syscall_get_error()
> >> returned nonzero
> >>
> >> Is it the same with regs_set_return_value(), only meaningfull where
> >> there is no error ?
> > 
> > Did you mean syscall_set_return_value?  No, it explicitly has two
> > arguments, "int error" and "long val", so it can be used to either
> > clear or set the error condition as specified by the caller.
> 
> Sorry, I mean syscall_get_return_value() here.
> 
> static inline long syscall_get_return_value(struct task_struct *task,
> 					    struct pt_regs *regs)
> {
> 	return regs->gpr[3];
> }
> 
> Versus
> 
> static inline void regs_set_return_value(struct pt_regs *regs, unsigned 
> long rc)
> {
> 	regs->gpr[3] = rc;
> }

The asm/syscall.h API provides two functions to obtain the return value:
syscall_get_error() and syscall_get_return_value().  The first one is used
to obtain the error code when the error condition is set.  When the error
condition is not set, it returns 0.  The second function is used to obtain
the return value when the error condition is not set.  When the error
condition is set, its return value is undefined.


-- 
ldv