Message ID | 20220829055424.19055-1-matthew.ruffell@canonical.com |
---|---|
Headers | show |
Series | LSM: Configuring Too Many LSMs Causes Kernel Panic on Boot | expand |
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@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
On 8/28/22 23:54, 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@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. > > 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(-) > Acked-by: Tim Gardner <tim.gardner@canonical.com> Looks reasonable.