Re: [PATCH RFC v15 12/21] security: add security_bdev_setintegrity() hook

Paul Moore
2024-03-19T23:00:38Z

On Mar 15, 2024 Fan Wu <wufan@linux.microsoft.com> wrote:
>
> This patch introduces a new hook to save block device's integrity
> data. For example, for dm-verity, LSMs can use this hook to save
> the roothash signature of a dm-verity into the security blob,
> and LSMs can make access decisions based on the data inside
> the signature, like the signer certificate.
>
> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
>
> --
> v1-v14:
> + Not present
>
> v15:
> + Introduced
>
> ---
> include/linux/lsm_hook_defs.h | 2 ++
> include/linux/security.h | 14 ++++++++++++++
> security/security.c | 28 ++++++++++++++++++++++++++++
> 3 files changed, 44 insertions(+)

I'm not sure why you made this a separate patch, help? If there is
no significant reason why this is separate, please squash it together
with patch 11/21.

> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index c335404470dc..6808ae763913 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -455,4 +455,6 @@ LSM_HOOK(void, LSM_RET_VOID, initramfs_populated, void)
>
> LSM_HOOK(int, 0, bdev_alloc_security, struct block_device *bdev)
> LSM_HOOK(void, LSM_RET_VOID, bdev_free_security, struct block_device *bdev)
> +LSM_HOOK(int, 0, bdev_setintegrity, struct block_device *bdev,
> + enum lsm_intgr_type type, const void *value, size_t size)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 9965b5c50df4..eaff8868766a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -83,6 +83,10 @@ enum lsm_event {
> LSM_POLICY_CHANGE,
> };
>
> +enum lsm_intgr_type {
> + __LSM_INTGR_MAX
> +};
> +
> /*
> * These are reasons that can be passed to the security_locked_down()
> * LSM hook. Lockdown reasons that protect kernel integrity (ie, the
> @@ -511,6 +515,9 @@ int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, u32 *uctx_len,
> void *val, size_t val_len, u64 id, u64 flags);
> int security_bdev_alloc(struct block_device *bdev);
> void security_bdev_free(struct block_device *bdev);
> +int security_bdev_setintegrity(struct block_device *bdev,
> + enum lsm_intgr_type type, const void *value,
> + size_t size);
> #else /* CONFIG_SECURITY */
>
> static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
> @@ -1495,6 +1502,13 @@ static inline void security_bdev_free(struct block_device *bdev)
> {
> }
>
> +static inline int security_bdev_setintegrity(struct block_device *bdev,
> + enum lsm_intgr_type, type,
> + const void *value, size_t size)
> +{
> + return 0;
> +}
> +
> #endif /* CONFIG_SECURITY */
>
> #if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
> diff --git a/security/security.c b/security/security.c
> index 4274bbee40d0..8d88529ac904 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -5637,6 +5637,34 @@ void security_bdev_free(struct block_device *bdev)
> }
> EXPORT_SYMBOL(security_bdev_free);
>
> +/**
> + * security_bdev_setintegrity() - Set the bdev's integrity data

Let's just say "Set the device's integrity data" and not ask people to
figure out "bdev", although I will admit it should be fairly obvious :)

> + * @bdev: block device
> + * @type: type of integrity, e.g. hash digest, signature, etc
> + * @value: the integrity value
> + * @size: size of the integrity value
> + *
> + * Register a verified integrity measurement of a bdev with the LSM.
> + *
> + * Return: Returns 0 on success, negative values on failure.
> + */
> +int security_bdev_setintegrity(struct block_device *bdev,
> + enum lsm_intgr_type type, const void *value,
> + size_t size)
> +{
> + int rc = 0;
> + struct security_hook_list *p;
> +
> + hlist_for_each_entry(p, &security_hook_heads.bdev_setintegrity, list) {
> + rc = p->hook.bdev_setintegrity(bdev, type, value, size);
> + if (rc)
> + return rc;
> + }
> +
> + return LSM_RET_DEFAULT(bdev_setintegrity);

We can just use the call_int_hook() macro here instead of open coding
everything, right?

> +}
> +EXPORT_SYMBOL(security_bdev_setintegrity);
> +
> #ifdef CONFIG_PERF_EVENTS
> /**
> * security_perf_event_open() - Check if a perf event open is allowed
> --
> 2.44.0

--
paul-moore.com