APPLIED[K][Unstable]: [SRU][J][K][Unstable][PATCH 0/1] LSM: Configuring Too Many LSMs Causes Kernel Panic on Boot

[Andrea Righi]

On Mon, Aug 29, 2022 at 05:54:23PM +1200, Matthew Ruffell wrote:
> BugLink: https://bugs.launchpad.net/bugs/1987998
> 
> [Impact]
> 
> The Ubuntu kernel carries an out of tree patchet, known as
> "LSM: Module stacking for AppArmor" upstream, to enable stackable LSMs for
> containers. The revision the Ubuntu kernel carries is an older one, from 2020,
> and has some slight divergences from the latest revision in development.
> 
> One such divergence, is support for Landlock as a stackable LSM. When the
> stackable LSM patchset was applied, Landlock was still in development and not
> mainlined yet, and wasn't present in the earlier revision of the 
> "LSM: Module stacking for AppArmor" patchset. Support for this was added by us.
> 
> There was a minor omission made during enabling support for Landlock. The LSM
> slot type was marked as LSMBLOB_NEEDED, when it should have been 
> LSMBLOB_NOT_NEEDED.
> 
> Landlock itself does not provide any of the hooks that use a struct lsmblob,
> such as secid_to_secctx, secctx_to_secid, inode_getsecid, cred_getsecid, 
> kernel_act_as task_getsecid_subj task_getsecid_obj and ipc_getsecid. 
> 
> When we set .slot = LSMBLOB_NEEDED, this indicates that we need an entry in
> struct lsmblob, and we need to increment LSMBLOB_ENTRIES by one to fit the entry
> into the secid array:
> 
> #define LSMBLOB_ENTRIES ( \
>        (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \
>        (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \
>        (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \
>        (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0))
> 
> struct lsmblob {
>        u32     secid[LSMBLOB_ENTRIES];
> };
> 
> Currently, we don't increment LSMBLOB_ENTRIES by one to make an entry for
> Landlock, so for the Ubuntu kernel, we can fit a maximum of two entries, one for
> Apparmor and one for bpf.
> 
> If you try and configure three LSMs like so and reboot:
> 
> GRUB_CMDLINE_LINUX_DEFAULT="lsm=landlock,bpf,apparmor"
> 
> You will receive the following panic:
> 
> 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. ]---
> 
> There is a check added in security_add_hooks() that makes sure that you cannot
> configure too many LSMs:
> 
> if (lsmid->slot == LSMBLOB_NEEDED) {
> 	if (lsm_slot >= LSMBLOB_ENTRIES)
> 		panic("%s Too many LSMs registered.\n", __func__);
> 	lsmid->slot = lsm_slot++;
> 	init_debug("%s assigned lsmblob slot %d\n", lsmid->lsm,
> 		   lsmid->slot);
> }
> 
> A workaround is to enable no more than 2 LSMs until this is fixed.
> 
> [Fix]
> 
> If you read the following mailing list thread on linux-security-modules from
> May 2021:
> 
> https://lore.kernel.org/selinux/202105141224.942DE93@keescook/T/
> 
> It is explained that Landlock does not provide any of the hooks that use a 
> struct lsmblob, such as secid_to_secctx, secctx_to_secid, inode_getsecid, 
> cred_getsecid, kernel_act_as task_getsecid_subj task_getsecid_obj and 
> ipc_getsecid. 
> 
> I verified this with:
> 
> ubuntu-jammy$ grep -Rin "secid_to_secctx" security/landlock/
> ubuntu-jammy$ grep -Rin "secctx_to_secid" security/landlock/
> ubuntu-jammy$ grep -Rin "inode_getsecid" security/landlock/
> ubuntu-jammy$ grep -Rin "cred_getsecid" security/landlock/
> ubuntu-jammy$ grep -Rin "kernel_act_as" security/landlock/
> ubuntu-jammy$ grep -Rin "task_getsecid_subj" security/landlock/
> ubuntu-jammy$ grep -Rin "task_getsecid_obj" security/landlock/
> ubuntu-jammy$ grep -Rin "ipc_getsecid" security/landlock/
> 
> The fix is to change Landlock from LSMBLOB_NEEDED to LSMBLOB_NOT_NEEDED.
> 
> Due to the "LSM: Module stacking for AppArmor" patchset being 25 patches long,
> it was impractical to revert just the below patch and reapply with the fix, due
> to a large amount of conflicts:
> 
> commit f17b27a2790e72198d2aaf45242453e5a9043049
> Author: Casey Schaufler <casey at schaufler-ca.com>
> Date:   Mon Aug 17 16:02:56 2020 -0700
> Subject: UBUNTU: SAUCE: LSM: Create and manage the lsmblob data structure.
> Link: https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/jammy/commit/?id=f17b27a2790e72198d2aaf45242453e5a9043049
> 
> So instead, I wrote up a fix that just changes the Landlock LSM slots to follow
> the latest upstream development, from V37 of the patchset:
> 
> https://lore.kernel.org/selinux/20220628005611.13106-4-casey@schaufler-ca.com/
> 
> I refactored the landlock_lsmid struct to only be in one place, and to be marked
> as extern from security/landlock/setup.h.
> 
> [Testcase]
> 
> Launch a Jammy or Kinetic VM.
> 
> 1. Edit /etc/default/grub and append the following to GRUB_CMDLINE_LINUX_DEFAULT:
> "lsm=landlock,bpf,apparmor" 
> 2. sudo update-grub
> 3. reboot
> 
> The system will panic on boot.
> 
> If you install the test kernel from the following ppa:
> 
> https://launchpad.net/~mruffell/+archive/ubuntu/sf343286-test
> 
> Instead of a panic occurring, the kernel should display all LSMs initialising, 
> and continue to boot.
> 
> [    0.288224] LSM: Security Framework initializing
> [    0.289457] landlock: Up and running.
> [    0.290290] LSM support for eBPF active
> [    0.291189] AppArmor: AppArmor initialized
> 
> [Where problems could occur]
> 
> The risk of regression in changing Landlock from LSMBLOB_NEEDED to 
> LSMBLOB_NOT_NEEDED is low, due to Landlock not needing a slot in the secid array
> in struct lsmblob in the first place.
> 
> The refactor is minor and unlikely to introduce any issues with Landlock or its
> security promises.
> 
> I feel that simply fixing this small bug is less regression risk than reverting
> the entire 25 patch patchset and applying the latest V37 upstream patchset,
> which has undergone significant changes from mid 2020. I think its best we
> consume the newer patchset once it makes its way into mainline in a future
> kernel instead.
> 
> If a regression were to occur, users could configure 2 LSMs instead of all 3,
> or not enable Landlock.

It looks like a reasonable cleanup, thanks for fixing this.

Applied to kinetic:linux and kinetic:linux-unstable.

-Andrea