NACK[J]: [SRU][J][K][Unstable][PATCH 1/1] UBUNTU: SAUCE: LSM: Change Landlock from LSMBLOB_NEEDED to LSMBLOB_NOT_NEEDED

[Andrea Righi]

On Fri, Sep 02, 2022 at 07:54:44AM +0200, Stefan Bader wrote:
> On 29.08.22 07:54, Matthew Ruffell wrote:
> > BugLink: https://bugs.launchpad.net/bugs/1987998
> > 
> > The Landlock LSM does not register any hooks which use struct lsmblob, and does
> > not require a slot in the secid array of struct lsmblob.
> > 
> > Change LSMBLOB_NEEDED to LSMBLOB_NOT_NEEDED.
> > 
> > This is required to fix a panic on boot where too many LSMs can be configured,
> > since while we currently mark Landlock as LSMBLOB_NEEDED, we do not actually
> > make LSMBLOB_ENTRIES large enough to fit it, and we panic when more than 2
> > LSMs are configured, like:
> > 
> > GRUB_CMDLINE_LINUX_DEFAULT="lsm=landlock,bpf,apparmor"
> > 
> > LSM: Security Framework initializing
> > landlock: Up and running.
> > LSM support for eBPF active
> > Kernel panic - not syncing: security_add_hooks Too many LSMs registered.
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.15.0-46-generic #49-Ubuntu
> > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014
> > Call Trace:
> >   <TASK>
> >   show_stack+0x52/0x5c
> >   dump_stack_lvl+0x4a/0x63
> >   dump_stack+0x10/0x16
> >   panic+0x149/0x321
> >   security_add_hooks+0x45/0x13a
> >   apparmor_init+0x189/0x1ef
> >   initialize_lsm+0x54/0x74
> >   ordered_lsm_init+0x379/0x392
> >   security_init+0x40/0x49
> >   start_kernel+0x466/0x4dc
> >   x86_64_start_reservations+0x24/0x2a
> >   x86_64_start_kernel+0xe4/0xef
> >   secondary_startup_64_no_verify+0xc2/0xcb
> >   </TASK>
> > ---[ end Kernel panic - not syncing: security_add_hooks Too many LSMs registered. ]---
> > 
> > Also refactor the Landlock support by going to just one struct lsm_id, and
> > extern it from setup.h, following upstream development.
> > 
> > Fixes: f17b27a2790e ("UBUNTU: SAUCE: LSM: Create and manage the lsmblob data structure.") ubuntu-jammy
> > Signed-off-by: Matthew Ruffell <matthew.ruffell at canonical.com>
> > ---
> 
> Forwarding feedback from security:
> 
> So unfortunately landlock does use LSMBLOBS in 5.15 it is using cred, inode
> and superblock blobs
> 
> see security/landlock/setup.c:
> 
> struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = {
>     .lbs_cred = sizeof(struct landlock_cred_security),
>     .lbs_inode = sizeof(struct landlock_inode_security),
>     .lbs_superblock = sizeof(struct landlock_superblock_security),
> };
> 
> so NAK this will break things.
> 
> We need to increase LSMBLOB_ENTRIES
> 
> -Stefan

Thanks for checking this Stefan. I also dropped this patch from
kinetic:linux and kinetic:linux-unstable.

-Andrea