Re: [PATCH v16 01/18] seccomp: Convert __secure_computing() to return boolean
Kevin Brodsky <[email protected]> Fri, 3 Jul 2026 13:59:21 +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 03/07/2026 12:27, Michal Suchánek wrote: > 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. Totally agreed, if we have the opportunity to rename a completely undescriptive function name like "secure_computing" we should take it. - Kevin