Re: [PATCH v16 01/18] seccomp: Convert __secure_computing() to return boolean

Michal Suchánek <[email protected]> Fri, 3 Jul 2026 12:27:26 +0200
Newsgroups gmane.linux.uml.devel,gmane.linux.ports.alpha,gmane.linux.kernel,gmane.linux.ports.arm.kernel,gmane.linux.kernel.mm,gmane.linux.ports.mips,gmane.linux.ports.parisc,gmane.linux.ports.ppc64.devel,gmane.linux.ports.riscv,gmane.linux.ports.sh.devel
Message-ID <[email protected]>
On Fri, Jul 03, 2026 at 11:00:11AM +0100, Mark Rutland wrote:
> On Fri, Jul 03, 2026 at 11:48:49AM +0200, Thomas Gleixner wrote:
> > On Fri, Jul 03 2026 at 09:51, Michal Suchánek wrote:
> > > On Mon, Jun 29, 2026 at 09:05:59PM +0800, Jinjie Ruan wrote:
> > >> -	if (secure_computing())
> > >> +	if (!secure_computing())
> > >>  		return -1;
> > >
> > > Hello,
> > >
> > > I am not fond of this logic inversion. The boolean is meaningless in
> > > itself.
> > >
> > > Previously -1 was used to indicate that the syscall was filtered but you
> > > chose to invert the logic choosing true to mean syscall was not filtered.
> > >
> > > You could choose true to mean that syscall was fitered avoiding this
> > > inversion.
> > 
> > That's just wrong. Boolean logic makes more sense with having
> > (!condition()). Just because the old 0/-1 nonsense had it the other way
> > round does not mean it has to stay that way.
> 
> 100% agreed!
> 
> Bikeshedding below; sorry.
> 
> I think the bigger problem is just that secure_computing() is a terrible
> name that does not express the intended semantic -- it's not clear
> whether "secure computing" means "seccomp permit the syscall" or
> "seccomp is enabled and some special rules now apply" or something else
> entirely.
> 
> If we're changing the return type, it might be worth renaming the
> function something like:
> 
> 	seccomp_permits_syscall()

Then not only it is clear which way the boolean value should be
interpreted, it also pervents the accidental inversion of existing
calls. Overall great.

Thanks

Michal