linux-kernel - Re: [PATCH] headers/cleanup.h: Fix if_not

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

Message-ID: <CAHk-=whn07tnDosPfn+UcAtWHBcLg=KqA16SHVv0GV4t8P1fHw@mail.gmail.com>
Date: Wed, 20 Nov 2024 10:19:45 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: David Lechner <dlechner@...libre.com>
Cc: Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org, 
	Peter Zijlstra <peterz@...radead.org>, Thomas Gleixner <tglx@...utronix.de>, Will Deacon <will@...nel.org>, 
	Waiman Long <longman@...hat.com>, Boqun Feng <boqun.feng@...il.com>, 
	Borislav Petkov <bp@...en8.de>
Subject: Re: [PATCH] headers/cleanup.h: Fix if_not_guard() fragility

On Wed, 20 Nov 2024 at 09:57, David Lechner <dlechner@...libre.com> wrote:
>
>         cond_guard(mutex_intr, &st->lock, &ret);
>         if (ret)
>                 return ret;

I'm not convinced that improves on anything.

You just replace one disgusting syntax with another, and force people
to have a variable that they may not want to have (even if they have
an error return variable, it might commonly be an error pointer, for
example)

I really think the basic issue is that "cond_guard" itself is a pretty
broken concept. It simply doesn't work very well in the C syntax.

I wish people just gave up on it entirely rather than try to work
around that fundamental fact.

Not that long ago, Mathieu wanted to introduce "inactive guards" for
some similar reasons - kind of "conditional guards, except the
conditional is outside the guard". And I pointed out that the fix was
to rewrite the disgusting code so that THEY WEREN'T NEEDED in the
place he wanted to use them. Rewriting things to "Just Don't Do That,
Then" actually just improved code entirely:

   https://lore.kernel.org/all/CAHk-=wgRefOSUy88-rcackyb4Ss3yYjuqS_TJRJwY_p7E3r0SA@mail.gmail.com/

and honestly, I suspect the same is often true of this whole
"if_not_guard()" thing. It's not *hugely* often needed, and I strongly
suspect that the explicitly scoped version would be a *lot* safer.

The "if_not_guard()" model may be great for mindless conversions of
existing code. But I'm not convinced it's a great interface in itself,
or that "mindless conversions" of conditional locking is actually a
good thing.

          Linus