Re: [PATCH v8 03/12] landlock: Replace union access_masks_all with helper functions

["Mickaël Salaün" <mic@digikod.net>]

On Mon, Mar 30, 2026 at 09:00:31PM +0200, Günther Noack wrote:
> On Mon, Mar 30, 2026 at 12:53:21PM +0200, Mickaël Salaün wrote:
> > On Mon, Mar 30, 2026 at 11:56:40AM +0200, Mickaël Salaün wrote:
> > > On Fri, Mar 27, 2026 at 05:48:28PM +0100, Günther Noack wrote:
> > > > * Stop using a union for access_masks_all.
> > > > * Expose helper functions for intersection checks and union operations.
> > > > 
> > > > The memory layout of bitfields is only loosely defined by the C
> > > > standard, so our static assertion that expects a fixed size was
> > > > brittle, and it broke on some compilers when we attempted to add a
> > > > 17th file system access right.
> > > > 
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202603261438.jBx2DGNe-lkp@intel.com/
> > > > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > > > ---
> > > >  security/landlock/access.h  | 21 ++++++++++++++-------
> > > >  security/landlock/cred.h    | 10 ++--------
> > > >  security/landlock/ruleset.h | 13 ++++---------
> > > >  3 files changed, 20 insertions(+), 24 deletions(-)
> > > 
> > > I'd prefer this approach:
> > > 
> > > diff --git a/security/landlock/access.h b/security/landlock/access.h
> > > index 89dc8e7b93da..bc9efbb5c900 100644
> > > --- a/security/landlock/access.h
> > > +++ b/security/landlock/access.h
> > > @@ -50,7 +50,7 @@ struct access_masks {
> > >         access_mask_t fs : LANDLOCK_NUM_ACCESS_FS;
> > >         access_mask_t net : LANDLOCK_NUM_ACCESS_NET;
> > >         access_mask_t scope : LANDLOCK_NUM_SCOPE;
> > > -};
> > > +} __packed;
> > 
> > Actually, we can just use '__packed __aligned(sizeof(u32))' and avoid
> > the static_assert change.  That would have no impact on x86, but pack it
> > on m68k.
> 
> Thanks, good catch (and thanks for pushing it to mic-next).
> Fingers crossed that this works on m68k.

So, this works!  I did some experiments with m68k and this architecture
is very special: it packs bitfields at byte granularity, not at
storage-unit granularity, except when the size of a bitfield is a
multiple of 8, in which case it aligns on this size.

I also look at the past versions of Landlock (in the stable branches),
and they are good because struct access_masks (and the related assert)
was introduced in v6.11 and fs was exactly 16 bits, which makes m68k
aligns on 2 bytes and then the size of the struct was 4 bytes.
Switching fs to 17 bits removes this optimization (I guess) and pack
(back) to 3 bytes, so recording more bits can take less space!

That's why we don't need a standalone fix to backport...