Re: [PATCH RFC v12 5/20] initramfs|security: Add security hook to initramfs unpack

[Paul Moore <paul@paul-moore.com>]

On Jan 30, 2024 Fan Wu <wufan@linux.microsoft.com> wrote:
> 
> This patch introduces a new hook to notify security system that the
> content of initramfs has been unpacked into the rootfs.
> 
> Upon receiving this notification, the security system can activate
> a policy to allow only files that originated from the initramfs to
> execute or load into kernel during the early stages of booting.
> 
> This approach is crucial for minimizing the attack surface by
> ensuring that only trusted files from the initramfs are operational
> in the critical boot phase.
> 
> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
> ---
> v1-v11:
>   + Not present
> 
> v12:
>   + Introduced
> ---
>  include/linux/lsm_hook_defs.h |  4 ++++
>  include/linux/security.h      | 10 ++++++++++
>  init/initramfs.c              |  3 +++
>  security/security.c           | 12 ++++++++++++
>  4 files changed, 29 insertions(+)
> 
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 185924c56378..b247388786a9 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -425,3 +425,7 @@ LSM_HOOK(int, 0, uring_override_creds, const struct cred *new)
>  LSM_HOOK(int, 0, uring_sqpoll, void)
>  LSM_HOOK(int, 0, uring_cmd, struct io_uring_cmd *ioucmd)
>  #endif /* CONFIG_IO_URING */
> +
> +#ifdef CONFIG_BLK_DEV_INITRD
> +LSM_HOOK(void, LSM_RET_VOID, unpack_initramfs_security, void)
> +#endif /* CONFIG_BLK_DEV_INITRD */

Let's just call it "unpack_initramfs", the "_security" part is somewhat
implied since we are talking about a LSM hook ;)

> diff --git a/init/initramfs.c b/init/initramfs.c
> index 76deb48c38cb..075a5794cde5 100644
> --- a/init/initramfs.c
> +++ b/init/initramfs.c
> @@ -18,6 +18,7 @@
>  #include <linux/init_syscalls.h>
>  #include <linux/task_work.h>
>  #include <linux/umh.h>
> +#include <linux/security.h>
>  
>  static __initdata bool csum_present;
>  static __initdata u32 io_csum;
> @@ -720,6 +721,8 @@ static void __init do_populate_rootfs(void *unused, async_cookie_t cookie)
>  #endif
>  	}
>  
> +	security_unpack_initramfs();

Given the caller, what do you think of changing the hook name to
"security_initramfs_populated()"?  I think this not only matches up
better with the caller, "do_populate_rootfs()", but since in using the
past tense we help indicate that this hook happens *after* the rootfs
is populated with the initramfs data.

>  done:
>  	/*
>  	 * If the initrd region is overlapped with crashkernel reserved region,
> diff --git a/security/security.c b/security/security.c
> index ddf2e69cf8f2..2a527d4c69bc 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -5581,3 +5581,15 @@ int security_uring_cmd(struct io_uring_cmd *ioucmd)
>  	return call_int_hook(uring_cmd, 0, ioucmd);
>  }
>  #endif /* CONFIG_IO_URING */
> +
> +#ifdef CONFIG_BLK_DEV_INITRD
> +/**
> + * security_unpack_initramfs() - Notify LSM that initramfs has been loaded
> + *
> + * Tells the LSM the initramfs has been unpacked into the rootfs.
> + */
> +void security_unpack_initramfs(void)
> +{
> +	call_void_hook(unpack_initramfs_security);
> +}
> +#endif /* CONFIG_BLK_DEV_INITRD */
> -- 
> 2.43.0

--
paul-moore.com