[PATCH 4/6] LSM: Infrastructure management of the dev_tun blob

Casey Schaufler casey at schaufler-ca.com
Tue Jul 9 23:11:36 UTC 2024


On 7/9/2024 4:01 PM, John Johansen wrote:
> On 7/8/24 14:39, Casey Schaufler wrote:
>> Move management of the dev_tun security blob out of the individual
>> security modules and into the LSM infrastructure.  The security modules
>> tell the infrastructure how much space they require at initialization.
>>
>> Signed-off-by: Casey Schaufler <casey at schaufler-ca.com>
>
> one issue below, besides Paul's point the rest of it looks good
>
>> ---
>>   include/linux/lsm_hook_defs.h     |  2 +-
>>   include/linux/lsm_hooks.h         |  1 +
>>   security/security.c               | 11 ++++++++++-
>>   security/selinux/hooks.c          | 22 ++++++----------------
>>   security/selinux/include/objsec.h |  6 ++++++
>>   5 files changed, 24 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/linux/lsm_hook_defs.h
>> b/include/linux/lsm_hook_defs.h
>> index 44488b1ab9a9..601e3092a7ed 100644
>> --- a/include/linux/lsm_hook_defs.h
>> +++ b/include/linux/lsm_hook_defs.h
>> @@ -352,7 +352,7 @@ LSM_HOOK(void, LSM_RET_VOID,
>> secmark_refcount_inc, void)
>>   LSM_HOOK(void, LSM_RET_VOID, secmark_refcount_dec, void)
>>   LSM_HOOK(void, LSM_RET_VOID, req_classify_flow, const struct
>> request_sock *req,
>>        struct flowi_common *flic)
>> -LSM_HOOK(int, 0, tun_dev_alloc_security, void **security)
>> +LSM_HOOK(int, 0, tun_dev_alloc_security, void *security)
>>   LSM_HOOK(void, LSM_RET_VOID, tun_dev_free_security, void *security)
>>   LSM_HOOK(int, 0, tun_dev_create, void)
>>   LSM_HOOK(int, 0, tun_dev_attach_queue, void *security)
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 7233bc0737be..0ff14ff128c8 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -80,6 +80,7 @@ struct lsm_blob_sizes {
>>       int    lbs_msg_msg;
>>       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 438ec6708eb3..2c9d075f5f92 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -234,6 +234,7 @@ static void __init lsm_set_blob_sizes(struct
>> lsm_blob_sizes *needed)
>>       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);
>> +    lsm_set_blob_size(&needed->lbs_tun_dev, &blob_sizes.lbs_tun_dev);
>>       lsm_set_blob_size(&needed->lbs_xattr_count,
>>                 &blob_sizes.lbs_xattr_count);
>>   }
>> @@ -412,6 +413,7 @@ static void __init ordered_lsm_init(void)
>>       init_debug("sock blob size       = %d\n", blob_sizes.lbs_sock);
>>       init_debug("superblock blob size = %d\n",
>> blob_sizes.lbs_superblock);
>>       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);
>>         /*
>> @@ -4851,7 +4853,13 @@ EXPORT_SYMBOL(security_secmark_refcount_dec);
>>    */
>>   int security_tun_dev_alloc_security(void **security)
>>   {
>> -    return call_int_hook(tun_dev_alloc_security, security);
>> +    int rc;
>> +
>> +    rc = lsm_blob_alloc(security, blob_sizes.lbs_tun_dev, GFP_KERNEL);
>> +    if (rc)
>> +        return rc;
>> +
>> +    return call_int_hook(tun_dev_alloc_security, *security);
>
> we either need to free the blob here or make a change to tun_net_init.
>
> if security_tun_dev_alloc fails it is not calling the corresponding
> security_tun_dev_free_security()

Yes indeed, I missed that one. Fix in v2.

>
>>   }
>>   EXPORT_SYMBOL(security_tun_dev_alloc_security);
>>   @@ -4864,6 +4872,7 @@ EXPORT_SYMBOL(security_tun_dev_alloc_security);
>>   void security_tun_dev_free_security(void *security)
>>   {
>>       call_void_hook(tun_dev_free_security, security);
>> +    kfree(security);
>>   }
>>   EXPORT_SYMBOL(security_tun_dev_free_security);
>>   diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index b3de2e941ef7..7f1d8358922a 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -5573,24 +5573,14 @@ static void selinux_req_classify_flow(const
>> struct request_sock *req,
>>       flic->flowic_secid = req->secid;
>>   }
>>   -static int selinux_tun_dev_alloc_security(void **security)
>> +static int selinux_tun_dev_alloc_security(void *security)
>>   {
>> -    struct tun_security_struct *tunsec;
>> +    struct tun_security_struct *tunsec = selinux_tun_dev(security);
>>   -    tunsec = kzalloc(sizeof(*tunsec), GFP_KERNEL);
>> -    if (!tunsec)
>> -        return -ENOMEM;
>>       tunsec->sid = current_sid();
>> -
>> -    *security = tunsec;
>>       return 0;
>>   }
>>   -static void selinux_tun_dev_free_security(void *security)
>> -{
>> -    kfree(security);
>> -}
>> -
>>   static int selinux_tun_dev_create(void)
>>   {
>>       u32 sid = current_sid();
>> @@ -5608,7 +5598,7 @@ static int selinux_tun_dev_create(void)
>>     static int selinux_tun_dev_attach_queue(void *security)
>>   {
>> -    struct tun_security_struct *tunsec = security;
>> +    struct tun_security_struct *tunsec = selinux_tun_dev(security);
>>         return avc_has_perm(current_sid(), tunsec->sid,
>> SECCLASS_TUN_SOCKET,
>>                   TUN_SOCKET__ATTACH_QUEUE, NULL);
>> @@ -5616,7 +5606,7 @@ static int selinux_tun_dev_attach_queue(void
>> *security)
>>     static int selinux_tun_dev_attach(struct sock *sk, void *security)
>>   {
>> -    struct tun_security_struct *tunsec = security;
>> +    struct tun_security_struct *tunsec = selinux_tun_dev(security);
>>       struct sk_security_struct *sksec = selinux_sock(sk);
>>         /* we don't currently perform any NetLabel based labeling
>> here and it
>> @@ -5634,7 +5624,7 @@ static int selinux_tun_dev_attach(struct sock
>> *sk, void *security)
>>     static int selinux_tun_dev_open(void *security)
>>   {
>> -    struct tun_security_struct *tunsec = security;
>> +    struct tun_security_struct *tunsec = selinux_tun_dev(security);
>>       u32 sid = current_sid();
>>       int err;
>>   @@ -6975,6 +6965,7 @@ struct lsm_blob_sizes selinux_blob_sizes
>> __ro_after_init = {
>>       .lbs_sock = sizeof(struct sk_security_struct),
>>       .lbs_superblock = sizeof(struct superblock_security_struct),
>>       .lbs_xattr_count = SELINUX_INODE_INIT_XATTRS,
>> +    .lbs_tun_dev = sizeof(struct tun_security_struct),
>>   };
>>     #ifdef CONFIG_PERF_EVENTS
>> @@ -7285,7 +7276,6 @@ static struct security_hook_list
>> selinux_hooks[] __ro_after_init = {
>>       LSM_HOOK_INIT(secmark_refcount_inc, selinux_secmark_refcount_inc),
>>       LSM_HOOK_INIT(secmark_refcount_dec, selinux_secmark_refcount_dec),
>>       LSM_HOOK_INIT(req_classify_flow, selinux_req_classify_flow),
>> -    LSM_HOOK_INIT(tun_dev_free_security,
>> selinux_tun_dev_free_security),
>>       LSM_HOOK_INIT(tun_dev_create, selinux_tun_dev_create),
>>       LSM_HOOK_INIT(tun_dev_attach_queue, selinux_tun_dev_attach_queue),
>>       LSM_HOOK_INIT(tun_dev_attach, selinux_tun_dev_attach),
>> diff --git a/security/selinux/include/objsec.h
>> b/security/selinux/include/objsec.h
>> index 83b9443d6919..461c6985977d 100644
>> --- a/security/selinux/include/objsec.h
>> +++ b/security/selinux/include/objsec.h
>> @@ -207,4 +207,10 @@ static inline struct sk_security_struct
>> *selinux_sock(const struct sock *sock)
>>       return sock->sk_security + selinux_blob_sizes.lbs_sock;
>>   }
>>   +static inline struct tun_security_struct *
>> +selinux_tun_dev(void *security)
>> +{
>> +    return security + selinux_blob_sizes.lbs_tun_dev;
>> +}
>> +
>>   #endif /* _SELINUX_OBJSEC_H_ */
>
>



More information about the Linux-security-module-archive mailing list