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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zz3Ni99LLGufmOjV@gmail.com>
Date: Wed, 20 Nov 2024 12:52:43 +0100
From: Ingo Molnar <mingo@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: 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>,
	David Lechner <dlechner@...libre.com>
Subject: Re: [PATCH] headers/cleanup.h: Fix if_not_guard() fragility


* Ingo Molnar <mingo@...nel.org> wrote:

> 
> * Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> 
> > On Mon, 18 Nov 2024 at 01:03, Ingo Molnar <mingo@...nel.org> wrote:
> > >
> > >  - <linux/cleanup.h>:
> > >     - Add if_not_cond_guard() conditional guard helper (David Lechner)
> > 
> > I've pulled this, but I'm unhappy.
> > 
> > This macro generates actively wrong code if it happens to be inside an
> > if-statement or a loop without a block.
> > 
> > IOW, code like this:
> > 
> >     for (iterate-over-something)
> >         if_not_guard(a)
> >             return -BUSY;
> > 
> > looks like will build fine, but will generate completely incorrect code.
> > 
> > Honestly, just switching the order of the BUILD_BUG_ON() and the
> > CLASS() declaration looks like it would have fixed this (because then
> > the '_id' won't be in scope of the subsequent if-statement any more),
> > but I'm unhappy with how apparently nobody even bothered to think
> > about such a fundamental issue with macros.
> > 
> > Macros that expand to statements absolutely *ALWAYS* need to deal with
> > "what if we're in a single-statement situation?"
> 
> How about the fix below?

I also reviewed our other similar macros in <linux/cleanup.h>:

 - scoped_guard() appears to be single-statement safe: it uses a for() 
   statement with a partial body with an open 'else' branch, so if this 
   macro is used within single statements the entire block will be part 
   of the 'else' statement.

 - scoped_cond_guard(): similar construct to scoped_guard().

 - The other remaining multi-statement macros are variable definition 
   macros (DEFINE_CLASS(), et al), which are typically used in file 
   scope or in header scope, and are not expected to be used in single 
   statements.

So it appears to me we should be OK wrt. this class of bugs?

Thanks,

	Ingo