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 | org.kernel.vger.linux-alpha,dev.linux.lists.loongarch,org.infradead.lists.linux-riscv,org.infradead.lists.linux-um,org.kernel.vger.linux-csky,org.kernel.vger.linux-kernel,org.kernel.vger.linux-m68k,org.kernel.vger.linux-mips,org.kernel.vger.linux-parisc,org.kernel.vger.linux-s390,org.kernel.vger.linux-sh,org.kvack.linux-mm,org.ozlabs.lists.linuxppc-dev |
|---|---|
| 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