Re: [PATCH v2 6/6] LSM: Infrastructure management of the perf_event security blob

[John Johansen <john.johansen@canonical.com>]

On 7/10/24 14:32, Casey Schaufler wrote:
> Move management of the perf_event->security blob out of the individual
> security modules and into the security infrastructure. Instead of
> allocating the blobs from within the modules the modules tell the
> infrastructure how much space is required, and the space is allocated
> there.  There are no longer any modules that require the perf_event_free()
> hook.  The hook definition has been removed.
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

looks good

Reviewed-by: John Johansen <john.johansen@canonical.com>

> ---
>   include/linux/lsm_hook_defs.h     |  1 -
>   include/linux/lsm_hooks.h         |  1 +
>   security/security.c               | 20 ++++++++++++++++++--
>   security/selinux/hooks.c          | 18 ++++--------------
>   security/selinux/include/objsec.h |  6 ++++++
>   5 files changed, 29 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 7c979137c0f2..658e4ba282e6 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -438,7 +438,6 @@ LSM_HOOK(int, 0, locked_down, enum lockdown_reason what)
>   #ifdef CONFIG_PERF_EVENTS
>   LSM_HOOK(int, 0, perf_event_open, struct perf_event_attr *attr, int type)
>   LSM_HOOK(int, 0, perf_event_alloc, struct perf_event *event)
> -LSM_HOOK(void, LSM_RET_VOID, perf_event_free, struct perf_event *event)
>   LSM_HOOK(int, 0, perf_event_read, struct perf_event *event)
>   LSM_HOOK(int, 0, perf_event_write, struct perf_event *event)
>   #endif /* CONFIG_PERF_EVENTS */
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index b6fc6ac88723..f1ca8082075a 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -79,6 +79,7 @@ struct lsm_blob_sizes {
>   	int	lbs_ipc;
>   	int	lbs_key;
>   	int	lbs_msg_msg;
> +	int	lbs_perf_event;
>   	int	lbs_task;
>   	int	lbs_xattr_count; /* number of xattr slots in new_xattrs array */
>   	int	lbs_tun_dev;
> diff --git a/security/security.c b/security/security.c
> index e8f34cbb1990..444b0ea28c04 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -28,6 +28,7 @@
>   #include <linux/xattr.h>
>   #include <linux/msg.h>
>   #include <linux/overflow.h>
> +#include <linux/perf_event.h>
>   #include <net/flow.h>
>   #include <net/sock.h>
>   
> @@ -230,6 +231,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
>   	lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
>   	lsm_set_blob_size(&needed->lbs_key, &blob_sizes.lbs_key);
>   	lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
> +	lsm_set_blob_size(&needed->lbs_perf_event, &blob_sizes.lbs_perf_event);
>   	lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
>   	lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
>   	lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
> @@ -412,6 +414,7 @@ static void __init ordered_lsm_init(void)
>   	init_debug("msg_msg blob size    = %d\n", blob_sizes.lbs_msg_msg);
>   	init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
>   	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
> +	init_debug("perf event blob size = %d\n", blob_sizes.lbs_perf_event);
>   	init_debug("task blob size       = %d\n", blob_sizes.lbs_task);
>   	init_debug("tun device blob size = %d\n", blob_sizes.lbs_tun_dev);
>   	init_debug("xattr slots          = %d\n", blob_sizes.lbs_xattr_count);
> @@ -5659,7 +5662,19 @@ int security_perf_event_open(struct perf_event_attr *attr, int type)
>    */
>   int security_perf_event_alloc(struct perf_event *event)
>   {
> -	return call_int_hook(perf_event_alloc, event);
> +	int rc;
> +
> +	rc = lsm_blob_alloc(&event->security, blob_sizes.lbs_perf_event,
> +			    GFP_KERNEL);
> +	if (rc)
> +		return rc;
> +
> +	rc = call_int_hook(perf_event_alloc, event);
> +	if (rc) {
> +		kfree(event->security);
> +		event->security = NULL;
> +	}
> +	return rc;
>   }
>   
>   /**
> @@ -5670,7 +5685,8 @@ int security_perf_event_alloc(struct perf_event *event)
>    */
>   void security_perf_event_free(struct perf_event *event)
>   {
> -	call_void_hook(perf_event_free, event);
> +	kfree(event->security);
> +	event->security = NULL;
>   }
>   
>   /**
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 11f4bdabda97..f42f6af55a73 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6950,6 +6950,9 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
>   	.lbs_ipc = sizeof(struct ipc_security_struct),
>   	.lbs_key = sizeof(struct key_security_struct),
>   	.lbs_msg_msg = sizeof(struct msg_security_struct),
> +#ifdef CONFIG_PERF_EVENTS
> +	.lbs_perf_event = sizeof(struct perf_event_security_struct),
> +#endif
>   	.lbs_sock = sizeof(struct sk_security_struct),
>   	.lbs_superblock = sizeof(struct superblock_security_struct),
>   	.lbs_xattr_count = SELINUX_INODE_INIT_XATTRS,
> @@ -6981,24 +6984,12 @@ static int selinux_perf_event_alloc(struct perf_event *event)
>   {
>   	struct perf_event_security_struct *perfsec;
>   
> -	perfsec = kzalloc(sizeof(*perfsec), GFP_KERNEL);
> -	if (!perfsec)
> -		return -ENOMEM;
> -
> +	perfsec = selinux_perf_event(event->security);
>   	perfsec->sid = current_sid();
> -	event->security = perfsec;
>   
>   	return 0;
>   }
>   
> -static void selinux_perf_event_free(struct perf_event *event)
> -{
> -	struct perf_event_security_struct *perfsec = event->security;
> -
> -	event->security = NULL;
> -	kfree(perfsec);
> -}
> -
>   static int selinux_perf_event_read(struct perf_event *event)
>   {
>   	struct perf_event_security_struct *perfsec = event->security;
> @@ -7310,7 +7301,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
>   
>   #ifdef CONFIG_PERF_EVENTS
>   	LSM_HOOK_INIT(perf_event_open, selinux_perf_event_open),
> -	LSM_HOOK_INIT(perf_event_free, selinux_perf_event_free),
>   	LSM_HOOK_INIT(perf_event_read, selinux_perf_event_read),
>   	LSM_HOOK_INIT(perf_event_write, selinux_perf_event_write),
>   #endif
> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
> index b1878f9395b5..d632a9180b41 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h
> @@ -219,4 +219,10 @@ selinux_ib(void *ib_sec)
>   	return ib_sec + selinux_blob_sizes.lbs_ib;
>   }
>   
> +static inline struct perf_event_security_struct *
> +selinux_perf_event(void *perf_event)
> +{
> +	return perf_event + selinux_blob_sizes.lbs_perf_event;
> +}
> +
>   #endif /* _SELINUX_OBJSEC_H_ */