APPLIED: [SRU][J][PATCH V2 0/1] LSM: Configuring Too Many LSMs Causes Kernel Panic on Boot
Stefan Bader
stefan.bader at canonical.com
Wed Oct 5 13:56:33 UTC 2022
On 27.09.22 11:31, 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.
>
> [Other Info]
>
> This patchset was originally NACKed by Stefan Bader on request of John
> Johansen due to a minor misunderstanding on how LSMBLOB_NEEDED worked with
> blob slot registration.
>
> ACK:
> https://lists.ubuntu.com/archives/kernel-team/2022-August/132898.html
> Applied:
> https://lists.ubuntu.com/archives/kernel-team/2022-August/132887.html
> NACK:
> https://lists.ubuntu.com/archives/kernel-team/2022-September/132965.html
> Revert:
> https://lists.ubuntu.com/archives/kernel-team/2022-September/132966.html
>
> This led to a off list discussion between myself, Stefan bader, Andrea Righi,
> John Johansen, Jay Vosburgh and Tim Gardner:
>
> Subject:
> Please Re-review LSM Stacking Patchset fix - Change Landlock from LSMBLOB_NEEDED
> to LSMBLOB_NOT_NEEDED
>
> The discussion uses largely information found in this SRU template, plus some
> additional information from mailing list threads highlighting why it is safe to
> move to LSMBLOB_NOT_NEEDED.
>
> From Matthew Ruffell
> https://pastebin.canonical.com/p/hxMgk2tDG9/
>
> From John Johansen
> https://pastebin.canonical.com/p/ZM4qCMrx2M/
> https://pastebin.canonical.com/p/q6485wDwdt/
>
> From Matthew Ruffell
> https://pastebin.canonical.com/p/y2HxnDMS6T/
>
> From Stefan Bader
> https://pastebin.canonical.com/p/CCvVh8SvhB/
>
> Now that the misunderstanding has been cleared up, and the patchset has been
> double checked and approved by the Security Team, resubmitting the initial
> patch as V2.
>
> Matthew Ruffell (1):
> UBUNTU: SAUCE: LSM: Change Landlock from LSMBLOB_NEEDED to
> LSMBLOB_NOT_NEEDED
>
> security/landlock/cred.c | 5 -----
> security/landlock/fs.c | 5 -----
> security/landlock/ptrace.c | 5 -----
> security/landlock/setup.c | 5 +++++
> security/landlock/setup.h | 1 +
> 5 files changed, 6 insertions(+), 15 deletions(-)
>
Applied to jammy:linux/master-next. Thanks.
-Stefan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20221005/25d584d6/attachment-0001.sig>
More information about the kernel-team
mailing list