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

[Stefan Bader]

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

>   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 */

-------------- 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/20220902/55063fcb/attachment.sig>