[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250423.phe6choh9AiN@digikod.net>
Date: Wed, 23 Apr 2025 20:38:25 +0200
From: Mickaël Salaün <mic@...ikod.net>
To: Kees Cook <kees@...nel.org>
Cc: Arnd Bergmann <arnd@...db.de>, linux <linux@...blig.org>,
Mark Brown <broonie@...nel.org>, WangYuli <wangyuli@...ontech.com>,
Günther Noack <gnoack@...gle.com>, Paul Moore <paul@...l-moore.com>,
James Morris <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>,
Linus Torvalds <torvalds@...ux-foundation.org>, linux-security-module@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] landlock: Work around randstruct unnamed static
initializer support
On Tue, Apr 22, 2025 at 07:59:07AM -0700, Kees Cook wrote:
> On Tue, Apr 22, 2025 at 02:53:05PM +0200, Arnd Bergmann wrote:
> > On Tue, Apr 22, 2025, at 14:25, Mickaël Salaün wrote:
> > > On Sun, Apr 20, 2025 at 05:08:59PM -0700, Kees Cook wrote:
> > >> Unnamed static initializers aren't supported by the randstruct GCC
> > >> plugin. Quoting the plugin, "set up a bogus anonymous struct field
> > >> designed to error out on unnamed struct initializers as gcc provides
> > >> no other way to detect such code". That is exactly what happens
> > >> with the landlock code, so adjust the static initializers for structs
> > >> lsm_ioctlop_audit and landlock_request that contain a randomized structure
> > >> (struct path) to use named variables, which avoids the intentional
> > >> GCC crashes:
> > >
> > > This is not a sustainable solution. Could we fix the plugin instead?
> > > This new Landlock change may be the first to trigger this plugin bug but
> > > it will probably not be the last to use unnamed static initializers.
> > > Forbidding specific C constructs should be documented.
> >
> > I think the version from Kees' patch looks more readable than
> > the version with the compound literal, so it certainly seems appropriate
> > as an immediate regression fix, even if it's possible to fix the
> > plugin later.
> >
> > >> We went 8 years before tripping over this!
> >
> > Right, it's probably enough to revisit the plugin code after
> > it happens again.
>
> Yeah, that's my thinking as well.
>
> > >> Closes: https://lore.kernel.org/lkml/337D5D4887277B27+3c677db3-a8b9-47f0-93a4-7809355f1381@uniontech.com/
> > >> Signed-off-by: Kees Cook <kees@...nel.org>
> >
> > Acked-by: Arnd Bergmann <arnd@...db.de>
>
> Thanks! Mickaël, are you good with this for now, and if so, do you want
> to carry it or shall I?
I don't like this kind of change but I'm OK with it if it comes with
documentation explaining what construct should not be used in the kernel
(and even better, updating checkpatch.pl accordingly), or if it's a
temporary workaround and someone is working on fixing the plugin. Are
you or someone else working on it? In the meantime, feel free to carry
this patch.
Powered by blists - more mailing lists