| Message ID | 20220829055424.19055-2-matthew.ruffell@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | LSM: Configuring Too Many LSMs Causes Kernel Panic on Boot | expand |
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@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 > 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(-) > > diff --git a/security/landlock/cred.c b/security/landlock/cred.c > index e3bd04cc7177..2eb1d65f10d6 100644 > --- a/security/landlock/cred.c > +++ b/security/landlock/cred.c > @@ -14,11 +14,6 @@ > #include "ruleset.h" > #include "setup.h" > > -static struct lsm_id landlock_lsmid __lsm_ro_after_init = { > - .lsm = "landlock", > - .slot = LSMBLOB_NEEDED > -}; > - > static int hook_cred_prepare(struct cred *const new, > const struct cred *const old, const gfp_t gfp) > { > diff --git a/security/landlock/fs.c b/security/landlock/fs.c > index b81db9d184bd..d8842a2ac58a 100644 > --- a/security/landlock/fs.c > +++ b/security/landlock/fs.c > @@ -37,11 +37,6 @@ > #include "ruleset.h" > #include "setup.h" > > -static struct lsm_id landlock_lsmid __lsm_ro_after_init = { > - .lsm = "landlock", > - .slot = LSMBLOB_NEEDED > -}; > - > /* Underlying object management */ > > static void release_inode(struct landlock_object *const object) > diff --git a/security/landlock/ptrace.c b/security/landlock/ptrace.c > index 0f3bb8ea12db..eab35808f395 100644 > --- a/security/landlock/ptrace.c > +++ b/security/landlock/ptrace.c > @@ -20,11 +20,6 @@ > #include "ruleset.h" > #include "setup.h" > > -static struct lsm_id landlock_lsmid __lsm_ro_after_init = { > - .lsm = "landlock", > - .slot = LSMBLOB_NEEDED > -}; > - > /** > * domain_scope_le - Checks domain ordering for scoped ptrace > * > diff --git a/security/landlock/setup.c b/security/landlock/setup.c > index f8e8e980454c..759e00b9436c 100644 > --- a/security/landlock/setup.c > +++ b/security/landlock/setup.c > @@ -23,6 +23,11 @@ struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = { > .lbs_superblock = sizeof(struct landlock_superblock_security), > }; > > +struct lsm_id landlock_lsmid __lsm_ro_after_init = { > + .lsm = LANDLOCK_NAME, > + .slot = LSMBLOB_NOT_NEEDED, > +}; > + > static int __init landlock_init(void) > { > landlock_add_cred_hooks(); > diff --git a/security/landlock/setup.h b/security/landlock/setup.h > index 1daffab1ab4b..38bce5b172dc 100644 > --- a/security/landlock/setup.h > +++ b/security/landlock/setup.h > @@ -14,5 +14,6 @@ > extern bool landlock_initialized; > > extern struct lsm_blob_sizes landlock_blob_sizes; > +extern struct lsm_id landlock_lsmid; > > #endif /* _SECURITY_LANDLOCK_SETUP_H */
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@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
Hi Stefan, Andrea,
If you look at security/landlock/setup.c at current mainline 6.0-rc3,
we see the same code:
20 struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = {
21 .lbs_cred = sizeof(struct landlock_cred_security),
22 .lbs_inode = sizeof(struct landlock_inode_security),
23 .lbs_superblock = sizeof(struct landlock_superblock_security),
24 };
If you look at Casey's V37 of the patchset from 27 June 2022:
https://lore.kernel.org/selinux/20220628005611.13106-4-casey@schaufler-ca.com/
diff --git a/security/landlock/setup.c b/security/landlock/setup.c
index f8e8e980454c..759e00b9436c 100644
--- a/security/landlock/setup.c
+++ b/security/landlock/setup.c
@@ -23,6 +23,11 @@ struct lsm_blob_sizes landlock_blob_sizes
__lsm_ro_after_init = {
.lbs_superblock = sizeof(struct landlock_superblock_security),
};
+struct lsm_id landlock_lsmid __lsm_ro_after_init = {
+ .lsm = LANDLOCK_NAME,
+ .slot = LSMBLOB_NOT_NEEDED,
+};
+
static int __init landlock_init(void)
{
landlock_add_cred_hooks();
It is still marked as LSMBLOB_NOT_NEEDED. The code hunk in question,
.lbs_superblock, is even visible.
Now, looking at the code in landlock_blob_sizes, it uses struct
landlock_cred_security, struct landlock_inode_security and struct
landlock_superblock_security, with no mention of struct lsmblob.
Reading Casey's response in:
>>> diff --git a/security/landlock/setup.c b/security/landlock/setup.c
>>> index f8e8e980454c..4a12666a4090 100644
>>> --- a/security/landlock/setup.c
>>> +++ b/security/landlock/setup.c
>>> @@ -23,6 +23,10 @@ struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = {
>>> .lbs_superblock = sizeof(struct landlock_superblock_security),
>>> };
>>>
>>> +struct lsm_id landlock_lsmid __lsm_ro_after_init = {
>>> + .lsm = LANDLOCK_NAME,
>> It is missing: .slot = LSMBLOB_NEEDED,
>
> Landlock does not provide any of the hooks that use a struct lsmblob.
> That would be secid_to_secctx, secctx_to_secid, inode_getsecid,
> cred_getsecid, kernel_act_as task_getsecid_subj task_getsecid_obj and
> ipc_getsecid. Setting .slot = LSMBLOB_NEEDED indicates that the LSM
> uses a slot in struct lsmblob. Landlock does not need a slot.
I still think that it should be safe to set Landlock to LSMBLOB_NOT_NEEDED.
I still think we should use the patch as-is for Kinetic and Unstable and onward.
I am open to incrementing LSMBLOB_ENTRIES for Jammy to keep regression
risk down,
but it should be unneccessary, and it deviates from upstream.
Do we have any plans to pick up the most recent V37 patchset, so we can follow
closer to upstream? Or are we going to keep the old patchset from 2020 until
this eventually gets mainlined?
Please re-review, and if you think we should just increment LSMBLOB_ENTRIES
instead, I will send a new patch.
Thanks,
Matthew
On Fri, Sep 2, 2022 at 7:32 PM Andrea Righi <andrea.righi@canonical.com> wrote:
>
> 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@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
Hi Stefan, Andrea,
Will you re-review the patch, or is there no chance for my appeal to
be accepted?
Would you like me to re-submit by incrementing LSMBLOB_ENTRIES
instead, something like below:
diff --git a/include/linux/security.h b/include/linux/security.h
index 5c1c4933d4b2..602c543fcb14 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -175,6 +175,7 @@ static inline void lsmcontext_init(struct
lsmcontext *cp, char *context,
(IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \
(IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \
(IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0) + \
(IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0))
struct lsmblob {
Thanks,
Matthew
On Fri, Sep 2, 2022 at 7:38 PM Matthew Ruffell
<matthew.ruffell@canonical.com> wrote:
>
> Hi Stefan, Andrea,
>
> If you look at security/landlock/setup.c at current mainline 6.0-rc3,
> we see the same code:
>
> 20 struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = {
> 21 .lbs_cred = sizeof(struct landlock_cred_security),
> 22 .lbs_inode = sizeof(struct landlock_inode_security),
> 23 .lbs_superblock = sizeof(struct landlock_superblock_security),
> 24 };
>
> If you look at Casey's V37 of the patchset from 27 June 2022:
>
> https://lore.kernel.org/selinux/20220628005611.13106-4-casey@schaufler-ca.com/
>
> diff --git a/security/landlock/setup.c b/security/landlock/setup.c
> index f8e8e980454c..759e00b9436c 100644
> --- a/security/landlock/setup.c
> +++ b/security/landlock/setup.c
> @@ -23,6 +23,11 @@ struct lsm_blob_sizes landlock_blob_sizes
> __lsm_ro_after_init = {
> .lbs_superblock = sizeof(struct landlock_superblock_security),
> };
>
> +struct lsm_id landlock_lsmid __lsm_ro_after_init = {
> + .lsm = LANDLOCK_NAME,
> + .slot = LSMBLOB_NOT_NEEDED,
> +};
> +
> static int __init landlock_init(void)
> {
> landlock_add_cred_hooks();
>
> It is still marked as LSMBLOB_NOT_NEEDED. The code hunk in question,
> .lbs_superblock, is even visible.
>
> Now, looking at the code in landlock_blob_sizes, it uses struct
> landlock_cred_security, struct landlock_inode_security and struct
> landlock_superblock_security, with no mention of struct lsmblob.
>
> Reading Casey's response in:
>
> >>> diff --git a/security/landlock/setup.c b/security/landlock/setup.c
> >>> index f8e8e980454c..4a12666a4090 100644
> >>> --- a/security/landlock/setup.c
> >>> +++ b/security/landlock/setup.c
> >>> @@ -23,6 +23,10 @@ struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = {
> >>> .lbs_superblock = sizeof(struct landlock_superblock_security),
> >>> };
> >>>
> >>> +struct lsm_id landlock_lsmid __lsm_ro_after_init = {
> >>> + .lsm = LANDLOCK_NAME,
> >> It is missing: .slot = LSMBLOB_NEEDED,
> >
> > Landlock does not provide any of the hooks that use a struct lsmblob.
> > That would be secid_to_secctx, secctx_to_secid, inode_getsecid,
> > cred_getsecid, kernel_act_as task_getsecid_subj task_getsecid_obj and
> > ipc_getsecid. Setting .slot = LSMBLOB_NEEDED indicates that the LSM
> > uses a slot in struct lsmblob. Landlock does not need a slot.
>
> I still think that it should be safe to set Landlock to LSMBLOB_NOT_NEEDED.
>
> I still think we should use the patch as-is for Kinetic and Unstable and onward.
>
> I am open to incrementing LSMBLOB_ENTRIES for Jammy to keep regression
> risk down,
> but it should be unneccessary, and it deviates from upstream.
>
> Do we have any plans to pick up the most recent V37 patchset, so we can follow
> closer to upstream? Or are we going to keep the old patchset from 2020 until
> this eventually gets mainlined?
>
> Please re-review, and if you think we should just increment LSMBLOB_ENTRIES
> instead, I will send a new patch.
>
> Thanks,
> Matthew
>
> On Fri, Sep 2, 2022 at 7:32 PM Andrea Righi <andrea.righi@canonical.com> wrote:
> >
> > 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@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
diff --git a/security/landlock/cred.c b/security/landlock/cred.c index e3bd04cc7177..2eb1d65f10d6 100644 --- a/security/landlock/cred.c +++ b/security/landlock/cred.c @@ -14,11 +14,6 @@ #include "ruleset.h" #include "setup.h" -static struct lsm_id landlock_lsmid __lsm_ro_after_init = { - .lsm = "landlock", - .slot = LSMBLOB_NEEDED -}; - static int hook_cred_prepare(struct cred *const new, const struct cred *const old, const gfp_t gfp) { diff --git a/security/landlock/fs.c b/security/landlock/fs.c index b81db9d184bd..d8842a2ac58a 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -37,11 +37,6 @@ #include "ruleset.h" #include "setup.h" -static struct lsm_id landlock_lsmid __lsm_ro_after_init = { - .lsm = "landlock", - .slot = LSMBLOB_NEEDED -}; - /* Underlying object management */ static void release_inode(struct landlock_object *const object) diff --git a/security/landlock/ptrace.c b/security/landlock/ptrace.c index 0f3bb8ea12db..eab35808f395 100644 --- a/security/landlock/ptrace.c +++ b/security/landlock/ptrace.c @@ -20,11 +20,6 @@ #include "ruleset.h" #include "setup.h" -static struct lsm_id landlock_lsmid __lsm_ro_after_init = { - .lsm = "landlock", - .slot = LSMBLOB_NEEDED -}; - /** * domain_scope_le - Checks domain ordering for scoped ptrace * diff --git a/security/landlock/setup.c b/security/landlock/setup.c index f8e8e980454c..759e00b9436c 100644 --- a/security/landlock/setup.c +++ b/security/landlock/setup.c @@ -23,6 +23,11 @@ struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = { .lbs_superblock = sizeof(struct landlock_superblock_security), }; +struct lsm_id landlock_lsmid __lsm_ro_after_init = { + .lsm = LANDLOCK_NAME, + .slot = LSMBLOB_NOT_NEEDED, +}; + static int __init landlock_init(void) { landlock_add_cred_hooks(); diff --git a/security/landlock/setup.h b/security/landlock/setup.h index 1daffab1ab4b..38bce5b172dc 100644 --- a/security/landlock/setup.h +++ b/security/landlock/setup.h @@ -14,5 +14,6 @@ extern bool landlock_initialized; extern struct lsm_blob_sizes landlock_blob_sizes; +extern struct lsm_id landlock_lsmid; #endif /* _SECURITY_LANDLOCK_SETUP_H */
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@canonical.com> --- 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(-)