From 2aff9d20d50ac45dd13a013ef5231f4fb8912356 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Wed, 10 Jul 2024 14:32:25 -0700 Subject: lsm: infrastructure management of the sock security Move management of the sock->sk_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. Acked-by: Paul Moore Reviewed-by: Kees Cook Reviewed-by: John Johansen Acked-by: Stephen Smalley Signed-off-by: Casey Schaufler [PM: subject tweak] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 80 ++++++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 43 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 55c78c318ccd7..e08ffb1ddd5a3 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4584,7 +4584,7 @@ static int socket_sockcreate_sid(const struct task_security_struct *tsec, static int sock_has_perm(struct sock *sk, u32 perms) { - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); struct common_audit_data ad; struct lsm_network_audit net; @@ -4652,7 +4652,7 @@ static int selinux_socket_post_create(struct socket *sock, int family, isec->initialized = LABEL_INITIALIZED; if (sock->sk) { - sksec = sock->sk->sk_security; + sksec = selinux_sock(sock->sk); sksec->sclass = sclass; sksec->sid = sid; /* Allows detection of the first association on this socket */ @@ -4668,8 +4668,8 @@ static int selinux_socket_post_create(struct socket *sock, int family, static int selinux_socket_socketpair(struct socket *socka, struct socket *sockb) { - struct sk_security_struct *sksec_a = socka->sk->sk_security; - struct sk_security_struct *sksec_b = sockb->sk->sk_security; + struct sk_security_struct *sksec_a = selinux_sock(socka->sk); + struct sk_security_struct *sksec_b = selinux_sock(sockb->sk); sksec_a->peer_sid = sksec_b->sid; sksec_b->peer_sid = sksec_a->sid; @@ -4684,7 +4684,7 @@ static int selinux_socket_socketpair(struct socket *socka, static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen) { struct sock *sk = sock->sk; - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); u16 family; int err; @@ -4824,7 +4824,7 @@ static int selinux_socket_connect_helper(struct socket *sock, struct sockaddr *address, int addrlen) { struct sock *sk = sock->sk; - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); int err; err = sock_has_perm(sk, SOCKET__CONNECT); @@ -5002,9 +5002,9 @@ static int selinux_socket_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk) { - struct sk_security_struct *sksec_sock = sock->sk_security; - struct sk_security_struct *sksec_other = other->sk_security; - struct sk_security_struct *sksec_new = newsk->sk_security; + struct sk_security_struct *sksec_sock = selinux_sock(sock); + struct sk_security_struct *sksec_other = selinux_sock(other); + struct sk_security_struct *sksec_new = selinux_sock(newsk); struct common_audit_data ad; struct lsm_network_audit net; int err; @@ -5033,8 +5033,8 @@ static int selinux_socket_unix_stream_connect(struct sock *sock, static int selinux_socket_unix_may_send(struct socket *sock, struct socket *other) { - struct sk_security_struct *ssec = sock->sk->sk_security; - struct sk_security_struct *osec = other->sk->sk_security; + struct sk_security_struct *ssec = selinux_sock(sock->sk); + struct sk_security_struct *osec = selinux_sock(other->sk); struct common_audit_data ad; struct lsm_network_audit net; @@ -5071,7 +5071,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb, u16 family) { int err = 0; - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); u32 sk_sid = sksec->sid; struct common_audit_data ad; struct lsm_network_audit net; @@ -5100,7 +5100,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb, static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) { int err, peerlbl_active, secmark_active; - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); u16 family = sk->sk_family; u32 sk_sid = sksec->sid; struct common_audit_data ad; @@ -5168,7 +5168,7 @@ static int selinux_socket_getpeersec_stream(struct socket *sock, int err = 0; char *scontext = NULL; u32 scontext_len; - struct sk_security_struct *sksec = sock->sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sock->sk); u32 peer_sid = SECSID_NULL; if (sksec->sclass == SECCLASS_UNIX_STREAM_SOCKET || @@ -5228,34 +5228,27 @@ static int selinux_socket_getpeersec_dgram(struct socket *sock, static int selinux_sk_alloc_security(struct sock *sk, int family, gfp_t priority) { - struct sk_security_struct *sksec; - - sksec = kzalloc(sizeof(*sksec), priority); - if (!sksec) - return -ENOMEM; + struct sk_security_struct *sksec = selinux_sock(sk); sksec->peer_sid = SECINITSID_UNLABELED; sksec->sid = SECINITSID_UNLABELED; sksec->sclass = SECCLASS_SOCKET; selinux_netlbl_sk_security_reset(sksec); - sk->sk_security = sksec; return 0; } static void selinux_sk_free_security(struct sock *sk) { - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); - sk->sk_security = NULL; selinux_netlbl_sk_security_free(sksec); - kfree(sksec); } static void selinux_sk_clone_security(const struct sock *sk, struct sock *newsk) { - struct sk_security_struct *sksec = sk->sk_security; - struct sk_security_struct *newsksec = newsk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); + struct sk_security_struct *newsksec = selinux_sock(newsk); newsksec->sid = sksec->sid; newsksec->peer_sid = sksec->peer_sid; @@ -5269,7 +5262,7 @@ static void selinux_sk_getsecid(const struct sock *sk, u32 *secid) if (!sk) *secid = SECINITSID_ANY_SOCKET; else { - const struct sk_security_struct *sksec = sk->sk_security; + const struct sk_security_struct *sksec = selinux_sock(sk); *secid = sksec->sid; } @@ -5279,7 +5272,7 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent) { struct inode_security_struct *isec = inode_security_novalidate(SOCK_INODE(parent)); - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); if (sk->sk_family == PF_INET || sk->sk_family == PF_INET6 || sk->sk_family == PF_UNIX) @@ -5296,7 +5289,7 @@ static int selinux_sctp_process_new_assoc(struct sctp_association *asoc, { struct sock *sk = asoc->base.sk; u16 family = sk->sk_family; - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); struct common_audit_data ad; struct lsm_network_audit net; int err; @@ -5351,7 +5344,7 @@ static int selinux_sctp_process_new_assoc(struct sctp_association *asoc, static int selinux_sctp_assoc_request(struct sctp_association *asoc, struct sk_buff *skb) { - struct sk_security_struct *sksec = asoc->base.sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(asoc->base.sk); u32 conn_sid; int err; @@ -5384,7 +5377,7 @@ static int selinux_sctp_assoc_request(struct sctp_association *asoc, static int selinux_sctp_assoc_established(struct sctp_association *asoc, struct sk_buff *skb) { - struct sk_security_struct *sksec = asoc->base.sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(asoc->base.sk); if (!selinux_policycap_extsockclass()) return 0; @@ -5483,8 +5476,8 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname, static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk, struct sock *newsk) { - struct sk_security_struct *sksec = sk->sk_security; - struct sk_security_struct *newsksec = newsk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); + struct sk_security_struct *newsksec = selinux_sock(newsk); /* If policy does not support SECCLASS_SCTP_SOCKET then call * the non-sctp clone version. @@ -5500,8 +5493,8 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk static int selinux_mptcp_add_subflow(struct sock *sk, struct sock *ssk) { - struct sk_security_struct *ssksec = ssk->sk_security; - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *ssksec = selinux_sock(ssk); + struct sk_security_struct *sksec = selinux_sock(sk); ssksec->sclass = sksec->sclass; ssksec->sid = sksec->sid; @@ -5516,7 +5509,7 @@ static int selinux_mptcp_add_subflow(struct sock *sk, struct sock *ssk) static int selinux_inet_conn_request(const struct sock *sk, struct sk_buff *skb, struct request_sock *req) { - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); int err; u16 family = req->rsk_ops->family; u32 connsid; @@ -5537,7 +5530,7 @@ static int selinux_inet_conn_request(const struct sock *sk, struct sk_buff *skb, static void selinux_inet_csk_clone(struct sock *newsk, const struct request_sock *req) { - struct sk_security_struct *newsksec = newsk->sk_security; + struct sk_security_struct *newsksec = selinux_sock(newsk); newsksec->sid = req->secid; newsksec->peer_sid = req->peer_secid; @@ -5554,7 +5547,7 @@ static void selinux_inet_csk_clone(struct sock *newsk, static void selinux_inet_conn_established(struct sock *sk, struct sk_buff *skb) { u16 family = sk->sk_family; - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); /* handle mapped IPv4 packets arriving via IPv6 sockets */ if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP)) @@ -5629,7 +5622,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 sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); /* we don't currently perform any NetLabel based labeling here and it * isn't clear that we would want to do so anyway; while we could apply @@ -5752,7 +5745,7 @@ static unsigned int selinux_ip_output(void *priv, struct sk_buff *skb, return NF_ACCEPT; /* standard practice, label using the parent socket */ - sksec = sk->sk_security; + sksec = selinux_sock(sk); sid = sksec->sid; } else sid = SECINITSID_KERNEL; @@ -5775,7 +5768,7 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb, sk = skb_to_full_sk(skb); if (sk == NULL) return NF_ACCEPT; - sksec = sk->sk_security; + sksec = selinux_sock(sk); ad_net_init_from_iif(&ad, &net, state->out->ifindex, state->pf); if (selinux_parse_skb(skb, &ad, NULL, 0, &proto)) @@ -5864,7 +5857,7 @@ static unsigned int selinux_ip_postroute(void *priv, u32 skb_sid; struct sk_security_struct *sksec; - sksec = sk->sk_security; + sksec = selinux_sock(sk); if (selinux_skb_peerlbl_sid(skb, family, &skb_sid)) return NF_DROP; /* At this point, if the returned skb peerlbl is SECSID_NULL @@ -5893,7 +5886,7 @@ static unsigned int selinux_ip_postroute(void *priv, } else { /* Locally generated packet, fetch the security label from the * associated socket. */ - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); peer_sid = sksec->sid; secmark_perm = PACKET__SEND; } @@ -5936,7 +5929,7 @@ static int selinux_netlink_send(struct sock *sk, struct sk_buff *skb) unsigned int data_len = skb->len; unsigned char *data = skb->data; struct nlmsghdr *nlh; - struct sk_security_struct *sksec = sk->sk_security; + struct sk_security_struct *sksec = selinux_sock(sk); u16 sclass = sksec->sclass; u32 perm; @@ -6994,6 +6987,7 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = { .lbs_inode = sizeof(struct inode_security_struct), .lbs_ipc = sizeof(struct ipc_security_struct), .lbs_msg_msg = sizeof(struct msg_security_struct), + .lbs_sock = sizeof(struct sk_security_struct), .lbs_superblock = sizeof(struct superblock_security_struct), .lbs_xattr_count = SELINUX_INODE_INIT_XATTRS, }; -- cgit v1.2.3 From 5f8d28f6d7d568dbbc8c5bce94894474c07afd4f Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Wed, 10 Jul 2024 14:32:26 -0700 Subject: lsm: infrastructure management of the key security blob Move management of the key->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 existing modules that require a key_free hook, so the call to it and the definition for it have been removed. Signed-off-by: Casey Schaufler Reviewed-by: John Johansen [PM: subject tweak] Signed-off-by: Paul Moore --- include/linux/lsm_hook_defs.h | 1 - include/linux/lsm_hooks.h | 1 + security/security.c | 39 +++++++++++++++++++++++++++++++++++++-- security/selinux/hooks.c | 21 ++++----------------- security/selinux/include/objsec.h | 7 +++++++ security/smack/smack.h | 7 +++++++ security/smack/smack_lsm.c | 31 +++++++++++++------------------ 7 files changed, 69 insertions(+), 38 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 855db460e08bc..8cc60644f3bd0 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -403,7 +403,6 @@ LSM_HOOK(int, 0, xfrm_decode_session, struct sk_buff *skb, u32 *secid, #ifdef CONFIG_KEYS LSM_HOOK(int, 0, key_alloc, struct key *key, const struct cred *cred, unsigned long flags) -LSM_HOOK(void, LSM_RET_VOID, key_free, struct key *key) LSM_HOOK(int, 0, key_permission, key_ref_t key_ref, const struct cred *cred, enum key_need_perm need_perm) LSM_HOOK(int, 0, key_getsecurity, struct key *key, char **buffer) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index efd4a0655159c..7233bc0737be6 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -76,6 +76,7 @@ struct lsm_blob_sizes { int lbs_sock; int lbs_superblock; int lbs_ipc; + int lbs_key; int lbs_msg_msg; int lbs_task; int lbs_xattr_count; /* number of xattr slots in new_xattrs array */ diff --git a/security/security.c b/security/security.c index 9c3fb2f60e2a6..dfb7fea6080c3 100644 --- a/security/security.c +++ b/security/security.c @@ -227,6 +227,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed) blob_sizes.lbs_inode = sizeof(struct rcu_head); lsm_set_blob_size(&needed->lbs_inode, &blob_sizes.lbs_inode); 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_sock, &blob_sizes.lbs_sock); lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock); @@ -402,6 +403,9 @@ static void __init ordered_lsm_init(void) init_debug("file blob size = %d\n", blob_sizes.lbs_file); init_debug("inode blob size = %d\n", blob_sizes.lbs_inode); init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc); +#ifdef CONFIG_KEYS + init_debug("key blob size = %d\n", blob_sizes.lbs_key); +#endif /* CONFIG_KEYS */ 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); @@ -718,6 +722,29 @@ static int lsm_ipc_alloc(struct kern_ipc_perm *kip) return 0; } +#ifdef CONFIG_KEYS +/** + * lsm_key_alloc - allocate a composite key blob + * @key: the key that needs a blob + * + * Allocate the key blob for all the modules + * + * Returns 0, or -ENOMEM if memory can't be allocated. + */ +static int lsm_key_alloc(struct key *key) +{ + if (blob_sizes.lbs_key == 0) { + key->security = NULL; + return 0; + } + + key->security = kzalloc(blob_sizes.lbs_key, GFP_KERNEL); + if (key->security == NULL) + return -ENOMEM; + return 0; +} +#endif /* CONFIG_KEYS */ + /** * lsm_msg_msg_alloc - allocate a composite msg_msg blob * @mp: the msg_msg that needs a blob @@ -5316,7 +5343,14 @@ EXPORT_SYMBOL(security_skb_classify_flow); int security_key_alloc(struct key *key, const struct cred *cred, unsigned long flags) { - return call_int_hook(key_alloc, key, cred, flags); + int rc = lsm_key_alloc(key); + + if (unlikely(rc)) + return rc; + rc = call_int_hook(key_alloc, key, cred, flags); + if (unlikely(rc)) + security_key_free(key); + return rc; } /** @@ -5327,7 +5361,8 @@ int security_key_alloc(struct key *key, const struct cred *cred, */ void security_key_free(struct key *key) { - call_void_hook(key_free, key); + kfree(key->security); + key->security = NULL; } /** diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index e08ffb1ddd5a3..7dbf169f733fd 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6663,11 +6663,7 @@ static int selinux_key_alloc(struct key *k, const struct cred *cred, unsigned long flags) { const struct task_security_struct *tsec; - struct key_security_struct *ksec; - - ksec = kzalloc(sizeof(struct key_security_struct), GFP_KERNEL); - if (!ksec) - return -ENOMEM; + struct key_security_struct *ksec = selinux_key(k); tsec = selinux_cred(cred); if (tsec->keycreate_sid) @@ -6675,18 +6671,9 @@ static int selinux_key_alloc(struct key *k, const struct cred *cred, else ksec->sid = tsec->sid; - k->security = ksec; return 0; } -static void selinux_key_free(struct key *k) -{ - struct key_security_struct *ksec = k->security; - - k->security = NULL; - kfree(ksec); -} - static int selinux_key_permission(key_ref_t key_ref, const struct cred *cred, enum key_need_perm need_perm) @@ -6727,14 +6714,14 @@ static int selinux_key_permission(key_ref_t key_ref, sid = cred_sid(cred); key = key_ref_to_ptr(key_ref); - ksec = key->security; + ksec = selinux_key(key); return avc_has_perm(sid, ksec->sid, SECCLASS_KEY, perm, NULL); } static int selinux_key_getsecurity(struct key *key, char **_buffer) { - struct key_security_struct *ksec = key->security; + struct key_security_struct *ksec = selinux_key(key); char *context = NULL; unsigned len; int rc; @@ -6986,6 +6973,7 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = { .lbs_file = sizeof(struct file_security_struct), .lbs_inode = sizeof(struct inode_security_struct), .lbs_ipc = sizeof(struct ipc_security_struct), + .lbs_key = sizeof(struct key_security_struct), .lbs_msg_msg = sizeof(struct msg_security_struct), .lbs_sock = sizeof(struct sk_security_struct), .lbs_superblock = sizeof(struct superblock_security_struct), @@ -7324,7 +7312,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { #endif #ifdef CONFIG_KEYS - LSM_HOOK_INIT(key_free, selinux_key_free), LSM_HOOK_INIT(key_permission, selinux_key_permission), LSM_HOOK_INIT(key_getsecurity, selinux_key_getsecurity), #ifdef CONFIG_KEY_NOTIFICATIONS diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index b074099acbaf7..83b9443d69190 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -195,6 +195,13 @@ selinux_superblock(const struct super_block *superblock) return superblock->s_security + selinux_blob_sizes.lbs_superblock; } +#ifdef CONFIG_KEYS +static inline struct key_security_struct *selinux_key(const struct key *key) +{ + return key->security + selinux_blob_sizes.lbs_key; +} +#endif /* CONFIG_KEYS */ + static inline struct sk_security_struct *selinux_sock(const struct sock *sock) { return sock->sk_security + selinux_blob_sizes.lbs_sock; diff --git a/security/smack/smack.h b/security/smack/smack.h index 297f21446f456..dbf8d7226eb56 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -360,6 +360,13 @@ static inline struct socket_smack *smack_sock(const struct sock *sock) return sock->sk_security + smack_blob_sizes.lbs_sock; } +#ifdef CONFIG_KEYS +static inline struct smack_known **smack_key(const struct key *key) +{ + return key->security + smack_blob_sizes.lbs_key; +} +#endif /* CONFIG_KEYS */ + /* * Is the directory transmuting? */ diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 742ccbd67a579..d690bfda893ac 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4504,23 +4504,13 @@ static void smack_inet_csk_clone(struct sock *sk, static int smack_key_alloc(struct key *key, const struct cred *cred, unsigned long flags) { + struct smack_known **blob = smack_key(key); struct smack_known *skp = smk_of_task(smack_cred(cred)); - key->security = skp; + *blob = skp; return 0; } -/** - * smack_key_free - Clear the key security blob - * @key: the object - * - * Clear the blob pointer - */ -static void smack_key_free(struct key *key) -{ - key->security = NULL; -} - /** * smack_key_permission - Smack access on a key * @key_ref: gets to the object @@ -4534,6 +4524,8 @@ static int smack_key_permission(key_ref_t key_ref, const struct cred *cred, enum key_need_perm need_perm) { + struct smack_known **blob; + struct smack_known *skp; struct key *keyp; struct smk_audit_info ad; struct smack_known *tkp = smk_of_task(smack_cred(cred)); @@ -4571,7 +4563,9 @@ static int smack_key_permission(key_ref_t key_ref, * If the key hasn't been initialized give it access so that * it may do so. */ - if (keyp->security == NULL) + blob = smack_key(keyp); + skp = *blob; + if (skp == NULL) return 0; /* * This should not occur @@ -4587,8 +4581,8 @@ static int smack_key_permission(key_ref_t key_ref, ad.a.u.key_struct.key = keyp->serial; ad.a.u.key_struct.key_desc = keyp->description; #endif - rc = smk_access(tkp, keyp->security, request, &ad); - rc = smk_bu_note("key access", tkp, keyp->security, request, rc); + rc = smk_access(tkp, skp, request, &ad); + rc = smk_bu_note("key access", tkp, skp, request, rc); return rc; } @@ -4603,11 +4597,12 @@ static int smack_key_permission(key_ref_t key_ref, */ static int smack_key_getsecurity(struct key *key, char **_buffer) { - struct smack_known *skp = key->security; + struct smack_known **blob = smack_key(key); + struct smack_known *skp = *blob; size_t length; char *copy; - if (key->security == NULL) { + if (skp == NULL) { *_buffer = NULL; return 0; } @@ -5041,6 +5036,7 @@ struct lsm_blob_sizes smack_blob_sizes __ro_after_init = { .lbs_file = sizeof(struct smack_known *), .lbs_inode = sizeof(struct inode_smack), .lbs_ipc = sizeof(struct smack_known *), + .lbs_key = sizeof(struct smack_known *), .lbs_msg_msg = sizeof(struct smack_known *), .lbs_sock = sizeof(struct socket_smack), .lbs_superblock = sizeof(struct superblock_smack), @@ -5178,7 +5174,6 @@ static struct security_hook_list smack_hooks[] __ro_after_init = { /* key management security hooks */ #ifdef CONFIG_KEYS LSM_HOOK_INIT(key_alloc, smack_key_alloc), - LSM_HOOK_INIT(key_free, smack_key_free), LSM_HOOK_INIT(key_permission, smack_key_permission), LSM_HOOK_INIT(key_getsecurity, smack_key_getsecurity), #ifdef CONFIG_KEY_NOTIFICATIONS -- cgit v1.2.3 From a39c0f77dbbe083f3eec6c3b32d90f168f7575eb Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Wed, 10 Jul 2024 14:32:28 -0700 Subject: lsm: infrastructure management of the dev_tun blob 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. There are no longer any modules that require the dev_tun_free hook. The hook definition has been removed. Signed-off-by: Casey Schaufler Reviewed-by: John Johansen [PM: subject tweak, selinux style fixes] Signed-off-by: Paul Moore --- include/linux/lsm_hook_defs.h | 3 +-- include/linux/lsm_hooks.h | 1 + security/security.c | 17 +++++++++++++++-- security/selinux/hooks.c | 22 ++++++---------------- security/selinux/include/objsec.h | 5 +++++ 5 files changed, 28 insertions(+), 20 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 8cc60644f3bd0..22c475f530aea 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -353,8 +353,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(void, LSM_RET_VOID, tun_dev_free_security, void *security) +LSM_HOOK(int, 0, tun_dev_alloc_security, void *security) LSM_HOOK(int, 0, tun_dev_create, void) LSM_HOOK(int, 0, tun_dev_attach_queue, void *security) LSM_HOOK(int, 0, tun_dev_attach, struct sock *sk, void *security) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 7233bc0737be6..0ff14ff128c80 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 15efcf43e1688..137a1f7e0e8bf 100644 --- a/security/security.c +++ b/security/security.c @@ -232,6 +232,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); } @@ -410,6 +411,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); /* @@ -4875,7 +4877,18 @@ 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; + + rc = call_int_hook(tun_dev_alloc_security, *security); + if (rc) { + kfree(*security); + *security = NULL; + } + return rc; } EXPORT_SYMBOL(security_tun_dev_alloc_security); @@ -4887,7 +4900,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 7dbf169f733fd..793cfdc4c0ef5 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5578,24 +5578,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(); @@ -5613,7 +5603,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); @@ -5621,7 +5611,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 @@ -5639,7 +5629,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; @@ -6978,6 +6968,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 @@ -7289,7 +7280,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 83b9443d69190..b7d4b1fc8feec 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -207,4 +207,9 @@ 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_ */ -- cgit v1.2.3 From 66de33a0bbb59ef3909d2c65dbbb7fc503d573bd Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Wed, 10 Jul 2024 14:32:29 -0700 Subject: lsm: infrastructure management of the infiniband blob Move management of the infiniband 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. There are no longer any modules that require the ib_free() hook. The hook definition has been removed. Signed-off-by: Casey Schaufler Reviewed-by: John Johansen [PM: subject tweak, selinux style fixes] Signed-off-by: Paul Moore --- include/linux/lsm_hook_defs.h | 3 +-- include/linux/lsm_hooks.h | 1 + security/security.c | 17 +++++++++++++++-- security/selinux/hooks.c | 16 +++------------- security/selinux/include/objsec.h | 5 +++++ 5 files changed, 25 insertions(+), 17 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 22c475f530aea..4fcbc5b3f58ce 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -373,8 +373,7 @@ LSM_HOOK(int, 0, mptcp_add_subflow, struct sock *sk, struct sock *ssk) LSM_HOOK(int, 0, ib_pkey_access, void *sec, u64 subnet_prefix, u16 pkey) LSM_HOOK(int, 0, ib_endport_manage_subnet, void *sec, const char *dev_name, u8 port_num) -LSM_HOOK(int, 0, ib_alloc_security, void **sec) -LSM_HOOK(void, LSM_RET_VOID, ib_free_security, void *sec) +LSM_HOOK(int, 0, ib_alloc_security, void *sec) #endif /* CONFIG_SECURITY_INFINIBAND */ #ifdef CONFIG_SECURITY_NETWORK_XFRM diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 0ff14ff128c80..b6fc6ac887238 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -72,6 +72,7 @@ struct security_hook_list { struct lsm_blob_sizes { int lbs_cred; int lbs_file; + int lbs_ib; int lbs_inode; int lbs_sock; int lbs_superblock; diff --git a/security/security.c b/security/security.c index 137a1f7e0e8bf..80dc58d60aabb 100644 --- a/security/security.c +++ b/security/security.c @@ -219,6 +219,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed) lsm_set_blob_size(&needed->lbs_cred, &blob_sizes.lbs_cred); lsm_set_blob_size(&needed->lbs_file, &blob_sizes.lbs_file); + lsm_set_blob_size(&needed->lbs_ib, &blob_sizes.lbs_ib); /* * The inode blob gets an rcu_head in addition to * what the modules might need. @@ -402,6 +403,7 @@ static void __init ordered_lsm_init(void) init_debug("cred blob size = %d\n", blob_sizes.lbs_cred); init_debug("file blob size = %d\n", blob_sizes.lbs_file); + init_debug("ib blob size = %d\n", blob_sizes.lbs_ib); init_debug("inode blob size = %d\n", blob_sizes.lbs_inode); init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc); #ifdef CONFIG_KEYS @@ -5096,7 +5098,18 @@ EXPORT_SYMBOL(security_ib_endport_manage_subnet); */ int security_ib_alloc_security(void **sec) { - return call_int_hook(ib_alloc_security, sec); + int rc; + + rc = lsm_blob_alloc(sec, blob_sizes.lbs_ib, GFP_KERNEL); + if (rc) + return rc; + + rc = call_int_hook(ib_alloc_security, *sec); + if (rc) { + kfree(*sec); + *sec = NULL; + } + return rc; } EXPORT_SYMBOL(security_ib_alloc_security); @@ -5108,7 +5121,7 @@ EXPORT_SYMBOL(security_ib_alloc_security); */ void security_ib_free_security(void *sec) { - call_void_hook(ib_free_security, sec); + kfree(sec); } EXPORT_SYMBOL(security_ib_free_security); #endif /* CONFIG_SECURITY_INFINIBAND */ diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 793cfdc4c0ef5..675c69ebb77c3 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6781,23 +6781,13 @@ static int selinux_ib_endport_manage_subnet(void *ib_sec, const char *dev_name, INFINIBAND_ENDPORT__MANAGE_SUBNET, &ad); } -static int selinux_ib_alloc_security(void **ib_sec) +static int selinux_ib_alloc_security(void *ib_sec) { - struct ib_security_struct *sec; + struct ib_security_struct *sec = selinux_ib(ib_sec); - sec = kzalloc(sizeof(*sec), GFP_KERNEL); - if (!sec) - return -ENOMEM; sec->sid = current_sid(); - - *ib_sec = sec; return 0; } - -static void selinux_ib_free_security(void *ib_sec) -{ - kfree(ib_sec); -} #endif #ifdef CONFIG_BPF_SYSCALL @@ -6969,6 +6959,7 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = { .lbs_superblock = sizeof(struct superblock_security_struct), .lbs_xattr_count = SELINUX_INODE_INIT_XATTRS, .lbs_tun_dev = sizeof(struct tun_security_struct), + .lbs_ib = sizeof(struct ib_security_struct), }; #ifdef CONFIG_PERF_EVENTS @@ -7288,7 +7279,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { LSM_HOOK_INIT(ib_pkey_access, selinux_ib_pkey_access), LSM_HOOK_INIT(ib_endport_manage_subnet, selinux_ib_endport_manage_subnet), - LSM_HOOK_INIT(ib_free_security, selinux_ib_free_security), #endif #ifdef CONFIG_SECURITY_NETWORK_XFRM LSM_HOOK_INIT(xfrm_policy_free_security, selinux_xfrm_policy_free), diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index b7d4b1fc8feec..ed9e37f3c9b55 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -212,4 +212,9 @@ static inline struct tun_security_struct *selinux_tun_dev(void *security) return security + selinux_blob_sizes.lbs_tun_dev; } +static inline struct ib_security_struct *selinux_ib(void *ib_sec) +{ + return ib_sec + selinux_blob_sizes.lbs_ib; +} + #endif /* _SELINUX_OBJSEC_H_ */ -- cgit v1.2.3 From 61a1dcdceb44d79e5ab511295791b88ea178c045 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Wed, 10 Jul 2024 14:32:30 -0700 Subject: lsm: infrastructure management of the perf_event security blob 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 Reviewed-by: John Johansen [PM: subject tweak] Signed-off-by: Paul Moore --- 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(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 4fcbc5b3f58ce..12c81be78eb94 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -439,7 +439,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 b6fc6ac887238..f1ca8082075a3 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 80dc58d60aabb..93ed7670fbc9f 100644 --- a/security/security.c +++ b/security/security.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -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); @@ -5685,7 +5688,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; } /** @@ -5696,7 +5711,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 675c69ebb77c3..0939816e96712 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6955,6 +6955,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, @@ -6986,24 +6989,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; @@ -7316,7 +7307,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 ed9e37f3c9b55..c88cae81ee4c5 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -217,4 +217,10 @@ static inline struct ib_security_struct *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_ */ -- cgit v1.2.3 From be72a57527fde6c80061c5f9d0e28762eb817b03 Mon Sep 17 00:00:00 2001 From: Xu Kuohai Date: Wed, 24 Jul 2024 10:06:58 +0800 Subject: lsm: Refactor return value of LSM hook vm_enough_memory To be consistent with most LSM hooks, convert the return value of hook vm_enough_memory to 0 or a negative error code. Before: - Hook vm_enough_memory returns 1 if permission is granted, 0 if not. - LSM_RET_DEFAULT(vm_enough_memory_mm) is 1. After: - Hook vm_enough_memory reutrns 0 if permission is granted, negative error code if not. - LSM_RET_DEFAULT(vm_enough_memory_mm) is 0. Signed-off-by: Xu Kuohai Reviewed-by: Casey Schaufler Signed-off-by: Paul Moore --- include/linux/lsm_hook_defs.h | 2 +- include/linux/security.h | 2 +- security/commoncap.c | 11 +++-------- security/security.c | 11 +++++------ security/selinux/hooks.c | 15 ++++----------- 5 files changed, 14 insertions(+), 27 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 12c81be78eb94..63e2656d1d567 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -48,7 +48,7 @@ LSM_HOOK(int, 0, quota_on, struct dentry *dentry) LSM_HOOK(int, 0, syslog, int type) LSM_HOOK(int, 0, settime, const struct timespec64 *ts, const struct timezone *tz) -LSM_HOOK(int, 1, vm_enough_memory, struct mm_struct *mm, long pages) +LSM_HOOK(int, 0, vm_enough_memory, struct mm_struct *mm, long pages) LSM_HOOK(int, 0, bprm_creds_for_exec, struct linux_binprm *bprm) LSM_HOOK(int, 0, bprm_creds_from_file, struct linux_binprm *bprm, const struct file *file) LSM_HOOK(int, 0, bprm_check_security, struct linux_binprm *bprm) diff --git a/include/linux/security.h b/include/linux/security.h index 1390f1efb4f07..62233fec8ead7 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -634,7 +634,7 @@ static inline int security_settime64(const struct timespec64 *ts, static inline int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) { - return __vm_enough_memory(mm, pages, cap_vm_enough_memory(mm, pages)); + return __vm_enough_memory(mm, pages, !cap_vm_enough_memory(mm, pages)); } static inline int security_bprm_creds_for_exec(struct linux_binprm *bprm) diff --git a/security/commoncap.c b/security/commoncap.c index 162d96b3a6768..cefad323a0b1b 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -1396,17 +1396,12 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3, * Determine whether the allocation of a new virtual mapping by the current * task is permitted. * - * Return: 1 if permission is granted, 0 if not. + * Return: 0 if permission granted, negative error code if not. */ int cap_vm_enough_memory(struct mm_struct *mm, long pages) { - int cap_sys_admin = 0; - - if (cap_capable(current_cred(), &init_user_ns, - CAP_SYS_ADMIN, CAP_OPT_NOAUDIT) == 0) - cap_sys_admin = 1; - - return cap_sys_admin; + return cap_capable(current_cred(), &init_user_ns, CAP_SYS_ADMIN, + CAP_OPT_NOAUDIT); } /** diff --git a/security/security.c b/security/security.c index 93ed7670fbc9f..b2f0e9a578640 100644 --- a/security/security.c +++ b/security/security.c @@ -1129,15 +1129,14 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) int rc; /* - * The module will respond with a positive value if - * it thinks the __vm_enough_memory() call should be - * made with the cap_sys_admin set. If all of the modules - * agree that it should be set it will. If any module - * thinks it should not be set it won't. + * The module will respond with 0 if it thinks the __vm_enough_memory() + * call should be made with the cap_sys_admin set. If all of the modules + * agree that it should be set it will. If any module thinks it should + * not be set it won't. */ hlist_for_each_entry(hp, &security_hook_heads.vm_enough_memory, list) { rc = hp->hook.vm_enough_memory(mm, pages); - if (rc <= 0) { + if (rc < 0) { cap_sys_admin = 0; break; } diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 0939816e96712..af7467cdd181f 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2202,23 +2202,16 @@ static int selinux_syslog(int type) } /* - * Check that a process has enough memory to allocate a new virtual - * mapping. 0 means there is enough memory for the allocation to - * succeed and -ENOMEM implies there is not. + * Check permission for allocating a new virtual mapping. Returns + * 0 if permission is granted, negative error code if not. * * Do not audit the selinux permission check, as this is applied to all * processes that allocate mappings. */ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages) { - int rc, cap_sys_admin = 0; - - rc = cred_has_capability(current_cred(), CAP_SYS_ADMIN, - CAP_OPT_NOAUDIT, true); - if (rc == 0) - cap_sys_admin = 1; - - return cap_sys_admin; + return cred_has_capability(current_cred(), CAP_SYS_ADMIN, + CAP_OPT_NOAUDIT, true); } /* binprm security operations */ -- cgit v1.2.3 From 924e19c39e8f0bbd581ab8a049f95a0ed02235b1 Mon Sep 17 00:00:00 2001 From: Xu Kuohai Date: Wed, 24 Jul 2024 10:06:59 +0800 Subject: lsm: Refactor return value of LSM hook inode_copy_up_xattr To be consistent with most LSM hooks, convert the return value of hook inode_copy_up_xattr to 0 or a negative error code. Before: - Hook inode_copy_up_xattr returns 0 when accepting xattr, 1 when discarding xattr, -EOPNOTSUPP if it does not know xattr, or any other negative error code otherwise. After: - Hook inode_copy_up_xattr returns 0 when accepting xattr, *-ECANCELED* when discarding xattr, -EOPNOTSUPP if it does not know xattr, or any other negative error code otherwise. Signed-off-by: Xu Kuohai Reviewed-by: Casey Schaufler Signed-off-by: Paul Moore --- fs/overlayfs/copy_up.c | 6 +++--- security/integrity/evm/evm_main.c | 2 +- security/security.c | 11 +++-------- security/selinux/hooks.c | 4 ++-- security/smack/smack_lsm.c | 6 +++--- 5 files changed, 12 insertions(+), 17 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index a5ef2005a2cc5..337a5be99ac9f 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -115,12 +115,12 @@ int ovl_copy_xattr(struct super_block *sb, const struct path *oldpath, struct de continue; error = security_inode_copy_up_xattr(old, name); - if (error < 0 && error != -EOPNOTSUPP) - break; - if (error == 1) { + if (error == -ECANCELED) { error = 0; continue; /* Discard */ } + if (error < 0 && error != -EOPNOTSUPP) + break; if (is_posix_acl_xattr(name)) { error = ovl_copy_acl(OVL_FS(sb), oldpath, new, name); diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 62fe66dd53ce5..6924ed508ebdb 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -1000,7 +1000,7 @@ static int evm_inode_copy_up_xattr(struct dentry *src, const char *name) case EVM_XATTR_HMAC: case EVM_IMA_XATTR_DIGSIG: default: - rc = 1; /* discard */ + rc = -ECANCELED; /* discard */ } kfree(xattr_data); diff --git a/security/security.c b/security/security.c index b2f0e9a578640..338e0d243a3c3 100644 --- a/security/security.c +++ b/security/security.c @@ -2674,19 +2674,14 @@ EXPORT_SYMBOL(security_inode_copy_up); * lower layer to the union/overlay layer. The caller is responsible for * reading and writing the xattrs, this hook is merely a filter. * - * Return: Returns 0 to accept the xattr, 1 to discard the xattr, -EOPNOTSUPP - * if the security module does not know about attribute, or a negative - * error code to abort the copy up. + * Return: Returns 0 to accept the xattr, -ECANCELED to discard the xattr, + * -EOPNOTSUPP if the security module does not know about attribute, + * or a negative error code to abort the copy up. */ int security_inode_copy_up_xattr(struct dentry *src, const char *name) { int rc; - /* - * The implementation can return 0 (accept the xattr), 1 (discard the - * xattr), -EOPNOTSUPP if it does not know anything about the xattr or - * any other error code in case of an error. - */ rc = call_int_hook(inode_copy_up_xattr, src, name); if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr)) return rc; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index af7467cdd181f..81fbfa5b80d4a 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3531,8 +3531,8 @@ static int selinux_inode_copy_up_xattr(struct dentry *dentry, const char *name) * xattrs up. Instead, filter out SELinux-related xattrs following * policy load. */ - if (selinux_initialized() && strcmp(name, XATTR_NAME_SELINUX) == 0) - return 1; /* Discard */ + if (selinux_initialized() && !strcmp(name, XATTR_NAME_SELINUX)) + return -ECANCELED; /* Discard */ /* * Any other attribute apart from SELINUX is not claimed, supported * by selinux. diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index d690bfda893ac..da0c2bffbd085 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4910,10 +4910,10 @@ static int smack_inode_copy_up(struct dentry *dentry, struct cred **new) static int smack_inode_copy_up_xattr(struct dentry *src, const char *name) { /* - * Return 1 if this is the smack access Smack attribute. + * Return -ECANCELED if this is the smack access Smack attribute. */ - if (strcmp(name, XATTR_NAME_SMACK) == 0) - return 1; + if (!strcmp(name, XATTR_NAME_SMACK)) + return -ECANCELED; return -EOPNOTSUPP; } -- cgit v1.2.3 From 2571bb9d553ba2b8db1971bd3c903bff07d0bb11 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Mon, 26 Aug 2024 08:47:09 -0400 Subject: selinux: annotate false positive data race to avoid KCSAN warnings KCSAN flags the check of isec->initialized by __inode_security_revalidate() as a data race. This is indeed a racy check, but inode_doinit_with_dentry() will recheck with isec->lock held. Annotate the check with the data_race() macro to silence the KCSAN false positive. Reported-by: syzbot+319ed1769c0078257262@syzkaller.appspotmail.com Signed-off-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 55c78c318ccd7..70c335846336d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -282,8 +282,13 @@ static int __inode_security_revalidate(struct inode *inode, might_sleep_if(may_sleep); + /* + * The check of isec->initialized below is racy but + * inode_doinit_with_dentry() will recheck with + * isec->lock held. + */ if (selinux_initialized() && - isec->initialized != LABEL_INITIALIZED) { + data_race(isec->initialized != LABEL_INITIALIZED)) { if (!may_sleep) return -ECHILD; -- cgit v1.2.3 From 1934b212615dc617ac84fc306333ab2b9fc3b04f Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Fri, 9 Aug 2024 18:00:01 +0200 Subject: file: reclaim 24 bytes from f_owner We do embedd struct fown_struct into struct file letting it take up 32 bytes in total. We could tweak struct fown_struct to be more compact but really it shouldn't even be embedded in struct file in the first place. Instead, actual users of struct fown_struct should allocate the struct on demand. This frees up 24 bytes in struct file. That will have some potentially user-visible changes for the ownership fcntl()s. Some of them can now fail due to allocation failures. Practically, that probably will almost never happen as the allocations are small and they only happen once per file. The fown_struct is used during kill_fasync() which is used by e.g., pipes to generate a SIGIO signal. Sending of such signals is conditional on userspace having set an owner for the file using one of the F_OWNER fcntl()s. Such users will be unaffected if struct fown_struct is allocated during the fcntl() call. There are a few subsystems that call __f_setown() expecting file->f_owner to be allocated: (1) tun devices file->f_op->fasync::tun_chr_fasync() -> __f_setown() There are no callers of tun_chr_fasync(). (2) tty devices file->f_op->fasync::tty_fasync() -> __tty_fasync() -> __f_setown() tty_fasync() has no additional callers but __tty_fasync() has. Note that __tty_fasync() only calls __f_setown() if the @on argument is true. It's called from: file->f_op->release::tty_release() -> tty_release() -> __tty_fasync() -> __f_setown() tty_release() calls __tty_fasync() with @on false => __f_setown() is never called from tty_release(). => All callers of tty_release() are safe as well. file->f_op->release::tty_open() -> tty_release() -> __tty_fasync() -> __f_setown() __tty_hangup() calls __tty_fasync() with @on false => __f_setown() is never called from tty_release(). => All callers of __tty_hangup() are safe as well. From the callchains it's obvious that (1) and (2) end up getting called via file->f_op->fasync(). That can happen either through the F_SETFL fcntl() with the FASYNC flag raised or via the FIOASYNC ioctl(). If FASYNC is requested and the file isn't already FASYNC then file->f_op->fasync() is called with @on true which ends up causing both (1) and (2) to call __f_setown(). (1) and (2) are the only subsystems that call __f_setown() from the file->f_op->fasync() handler. So both (1) and (2) have been updated to allocate a struct fown_struct prior to calling fasync_helper() to register with the fasync infrastructure. That's safe as they both call fasync_helper() which also does allocations if @on is true. The other interesting case are file leases: (3) file leases lease_manager_ops->lm_setup::lease_setup() -> __f_setown() Which in turn is called from: generic_add_lease() -> lease_manager_ops->lm_setup::lease_setup() -> __f_setown() So here again we can simply make generic_add_lease() allocate struct fown_struct prior to the lease_manager_ops->lm_setup::lease_setup() which happens under a spinlock. With that the two remaining subsystems that call __f_setown() are: (4) dnotify (5) sockets Both have their own custom ioctls to set struct fown_struct and both have been converted to allocate a struct fown_struct on demand from their respective ioctls. Interactions with O_PATH are fine as well e.g., when opening a /dev/tty as O_PATH then no file->f_op->open() happens thus no file->f_owner is allocated. That's fine as no file operation will be set for those and the device has never been opened. fcntl()s called on such things will just allocate a ->f_owner on demand. Although I have zero idea why'd you care about f_owner on an O_PATH fd. Link: https://lore.kernel.org/r/20240813-work-f_owner-v2-1-4e9343a79f9f@kernel.org Reviewed-by: Jeff Layton Signed-off-by: Christian Brauner --- drivers/net/tun.c | 6 ++ drivers/tty/tty_io.c | 6 ++ fs/fcntl.c | 166 +++++++++++++++++++++++++++++++++++--------- fs/file_table.c | 3 +- fs/internal.h | 1 + fs/locks.c | 6 +- fs/notify/dnotify/dnotify.c | 6 +- include/linux/fs.h | 11 ++- net/core/sock.c | 2 +- security/selinux/hooks.c | 2 +- security/smack/smack_lsm.c | 2 +- 11 files changed, 168 insertions(+), 43 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 1d06c560c5e65..6fe5e8f7017c4 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -3451,6 +3451,12 @@ static int tun_chr_fasync(int fd, struct file *file, int on) struct tun_file *tfile = file->private_data; int ret; + if (on) { + ret = file_f_owner_allocate(file); + if (ret) + goto out; + } + if ((ret = fasync_helper(fd, file, on, &tfile->fasync)) < 0) goto out; diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 407b0d87b7c10..7ae0c8934f423 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -2225,6 +2225,12 @@ static int __tty_fasync(int fd, struct file *filp, int on) if (tty_paranoia_check(tty, file_inode(filp), "tty_fasync")) goto out; + if (on) { + retval = file_f_owner_allocate(filp); + if (retval) + goto out; + } + retval = fasync_helper(fd, filp, on, &tty->fasync); if (retval <= 0) goto out; diff --git a/fs/fcntl.c b/fs/fcntl.c index 300e5d9ad913b..0587a0e221a6d 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -33,6 +33,8 @@ #include #include +#include "internal.h" + #define SETFL_MASK (O_APPEND | O_NONBLOCK | O_NDELAY | O_DIRECT | O_NOATIME) static int setfl(int fd, struct file * filp, unsigned int arg) @@ -87,22 +89,64 @@ static int setfl(int fd, struct file * filp, unsigned int arg) return error; } +/* + * Allocate an file->f_owner struct if it doesn't exist, handling racing + * allocations correctly. + */ +int file_f_owner_allocate(struct file *file) +{ + struct fown_struct *f_owner; + + f_owner = file_f_owner(file); + if (f_owner) + return 0; + + f_owner = kzalloc(sizeof(struct fown_struct), GFP_KERNEL); + if (!f_owner) + return -ENOMEM; + + rwlock_init(&f_owner->lock); + f_owner->file = file; + /* If someone else raced us, drop our allocation. */ + if (unlikely(cmpxchg(&file->f_owner, NULL, f_owner))) + kfree(f_owner); + return 0; +} +EXPORT_SYMBOL(file_f_owner_allocate); + +void file_f_owner_release(struct file *file) +{ + struct fown_struct *f_owner; + + f_owner = file_f_owner(file); + if (f_owner) { + put_pid(f_owner->pid); + kfree(f_owner); + } +} + static void f_modown(struct file *filp, struct pid *pid, enum pid_type type, int force) { - write_lock_irq(&filp->f_owner.lock); - if (force || !filp->f_owner.pid) { - put_pid(filp->f_owner.pid); - filp->f_owner.pid = get_pid(pid); - filp->f_owner.pid_type = type; + struct fown_struct *f_owner; + + f_owner = file_f_owner(filp); + if (WARN_ON_ONCE(!f_owner)) + return; + + write_lock_irq(&f_owner->lock); + if (force || !f_owner->pid) { + put_pid(f_owner->pid); + f_owner->pid = get_pid(pid); + f_owner->pid_type = type; if (pid) { const struct cred *cred = current_cred(); - filp->f_owner.uid = cred->uid; - filp->f_owner.euid = cred->euid; + f_owner->uid = cred->uid; + f_owner->euid = cred->euid; } } - write_unlock_irq(&filp->f_owner.lock); + write_unlock_irq(&f_owner->lock); } void __f_setown(struct file *filp, struct pid *pid, enum pid_type type, @@ -119,6 +163,8 @@ int f_setown(struct file *filp, int who, int force) struct pid *pid = NULL; int ret = 0; + might_sleep(); + type = PIDTYPE_TGID; if (who < 0) { /* avoid overflow below */ @@ -129,6 +175,10 @@ int f_setown(struct file *filp, int who, int force) who = -who; } + ret = file_f_owner_allocate(filp); + if (ret) + return ret; + rcu_read_lock(); if (who) { pid = find_vpid(who); @@ -152,16 +202,21 @@ void f_delown(struct file *filp) pid_t f_getown(struct file *filp) { pid_t pid = 0; + struct fown_struct *f_owner; + + f_owner = file_f_owner(filp); + if (!f_owner) + return pid; - read_lock_irq(&filp->f_owner.lock); + read_lock_irq(&f_owner->lock); rcu_read_lock(); - if (pid_task(filp->f_owner.pid, filp->f_owner.pid_type)) { - pid = pid_vnr(filp->f_owner.pid); - if (filp->f_owner.pid_type == PIDTYPE_PGID) + if (pid_task(f_owner->pid, f_owner->pid_type)) { + pid = pid_vnr(f_owner->pid); + if (f_owner->pid_type == PIDTYPE_PGID) pid = -pid; } rcu_read_unlock(); - read_unlock_irq(&filp->f_owner.lock); + read_unlock_irq(&f_owner->lock); return pid; } @@ -194,6 +249,10 @@ static int f_setown_ex(struct file *filp, unsigned long arg) return -EINVAL; } + ret = file_f_owner_allocate(filp); + if (ret) + return ret; + rcu_read_lock(); pid = find_vpid(owner.pid); if (owner.pid && !pid) @@ -210,13 +269,20 @@ static int f_getown_ex(struct file *filp, unsigned long arg) struct f_owner_ex __user *owner_p = (void __user *)arg; struct f_owner_ex owner = {}; int ret = 0; + struct fown_struct *f_owner; + enum pid_type pid_type = PIDTYPE_PID; - read_lock_irq(&filp->f_owner.lock); - rcu_read_lock(); - if (pid_task(filp->f_owner.pid, filp->f_owner.pid_type)) - owner.pid = pid_vnr(filp->f_owner.pid); - rcu_read_unlock(); - switch (filp->f_owner.pid_type) { + f_owner = file_f_owner(filp); + if (f_owner) { + read_lock_irq(&f_owner->lock); + rcu_read_lock(); + if (pid_task(f_owner->pid, f_owner->pid_type)) + owner.pid = pid_vnr(f_owner->pid); + rcu_read_unlock(); + pid_type = f_owner->pid_type; + } + + switch (pid_type) { case PIDTYPE_PID: owner.type = F_OWNER_TID; break; @@ -234,7 +300,8 @@ static int f_getown_ex(struct file *filp, unsigned long arg) ret = -EINVAL; break; } - read_unlock_irq(&filp->f_owner.lock); + if (f_owner) + read_unlock_irq(&f_owner->lock); if (!ret) { ret = copy_to_user(owner_p, &owner, sizeof(owner)); @@ -248,14 +315,18 @@ static int f_getown_ex(struct file *filp, unsigned long arg) static int f_getowner_uids(struct file *filp, unsigned long arg) { struct user_namespace *user_ns = current_user_ns(); + struct fown_struct *f_owner; uid_t __user *dst = (void __user *)arg; - uid_t src[2]; + uid_t src[2] = {0, 0}; int err; - read_lock_irq(&filp->f_owner.lock); - src[0] = from_kuid(user_ns, filp->f_owner.uid); - src[1] = from_kuid(user_ns, filp->f_owner.euid); - read_unlock_irq(&filp->f_owner.lock); + f_owner = file_f_owner(filp); + if (f_owner) { + read_lock_irq(&f_owner->lock); + src[0] = from_kuid(user_ns, f_owner->uid); + src[1] = from_kuid(user_ns, f_owner->euid); + read_unlock_irq(&f_owner->lock); + } err = put_user(src[0], &dst[0]); err |= put_user(src[1], &dst[1]); @@ -343,6 +414,30 @@ static long f_dupfd_query(int fd, struct file *filp) return f.file == filp; } +static int f_owner_sig(struct file *filp, int signum, bool setsig) +{ + int ret = 0; + struct fown_struct *f_owner; + + might_sleep(); + + if (setsig) { + if (!valid_signal(signum)) + return -EINVAL; + + ret = file_f_owner_allocate(filp); + if (ret) + return ret; + } + + f_owner = file_f_owner(filp); + if (setsig) + f_owner->signum = signum; + else if (f_owner) + ret = f_owner->signum; + return ret; +} + static long do_fcntl(int fd, unsigned int cmd, unsigned long arg, struct file *filp) { @@ -421,15 +516,10 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg, err = f_getowner_uids(filp, arg); break; case F_GETSIG: - err = filp->f_owner.signum; + err = f_owner_sig(filp, 0, false); break; case F_SETSIG: - /* arg == 0 restores default behaviour. */ - if (!valid_signal(argi)) { - break; - } - err = 0; - filp->f_owner.signum = argi; + err = f_owner_sig(filp, argi, true); break; case F_GETLEASE: err = fcntl_getlease(filp); @@ -844,14 +934,19 @@ static void send_sigurg_to_task(struct task_struct *p, do_send_sig_info(SIGURG, SEND_SIG_PRIV, p, type); } -int send_sigurg(struct fown_struct *fown) +int send_sigurg(struct file *file) { + struct fown_struct *fown; struct task_struct *p; enum pid_type type; struct pid *pid; unsigned long flags; int ret = 0; + fown = file_f_owner(file); + if (!fown) + return 0; + read_lock_irqsave(&fown->lock, flags); type = fown->pid_type; @@ -1027,13 +1122,16 @@ static void kill_fasync_rcu(struct fasync_struct *fa, int sig, int band) } read_lock_irqsave(&fa->fa_lock, flags); if (fa->fa_file) { - fown = &fa->fa_file->f_owner; + fown = file_f_owner(fa->fa_file); + if (!fown) + goto next; /* Don't send SIGURG to processes which have not set a queued signum: SIGURG has its own default signalling mechanism. */ if (!(sig == SIGURG && fown->signum == 0)) send_sigio(fown, fa->fa_fd, band); } +next: read_unlock_irqrestore(&fa->fa_lock, flags); fa = rcu_dereference(fa->fa_next); } diff --git a/fs/file_table.c b/fs/file_table.c index ca7843dde56dd..fdf98709dde23 100644 --- a/fs/file_table.c +++ b/fs/file_table.c @@ -155,7 +155,6 @@ static int init_file(struct file *f, int flags, const struct cred *cred) return error; } - rwlock_init(&f->f_owner.lock); spin_lock_init(&f->f_lock); mutex_init(&f->f_pos_lock); f->f_flags = flags; @@ -425,7 +424,7 @@ static void __fput(struct file *file) cdev_put(inode->i_cdev); } fops_put(file->f_op); - put_pid(file->f_owner.pid); + file_f_owner_release(file); put_file_access(file); dput(dentry); if (unlikely(mode & FMODE_NEED_UNMOUNT)) diff --git a/fs/internal.h b/fs/internal.h index cdd73209eecb4..8c1b7acbbe8fa 100644 --- a/fs/internal.h +++ b/fs/internal.h @@ -337,3 +337,4 @@ static inline bool path_mounted(const struct path *path) { return path->mnt->mnt_root == path->dentry; } +void file_f_owner_release(struct file *file); diff --git a/fs/locks.c b/fs/locks.c index e45cad40f8b6b..b51b1c395ce68 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -1451,7 +1451,7 @@ int lease_modify(struct file_lease *fl, int arg, struct list_head *dispose) struct file *filp = fl->c.flc_file; f_delown(filp); - filp->f_owner.signum = 0; + file_f_owner(filp)->signum = 0; fasync_helper(0, fl->c.flc_file, 0, &fl->fl_fasync); if (fl->fl_fasync != NULL) { printk(KERN_ERR "locks_delete_lock: fasync == %p\n", fl->fl_fasync); @@ -1783,6 +1783,10 @@ generic_add_lease(struct file *filp, int arg, struct file_lease **flp, void **pr lease = *flp; trace_generic_add_lease(inode, lease); + error = file_f_owner_allocate(filp); + if (error) + return error; + /* Note that arg is never F_UNLCK here */ ctx = locks_get_lock_context(inode, arg); if (!ctx) diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c index f3669403fabf1..46440fbb8662d 100644 --- a/fs/notify/dnotify/dnotify.c +++ b/fs/notify/dnotify/dnotify.c @@ -110,7 +110,7 @@ static int dnotify_handle_event(struct fsnotify_mark *inode_mark, u32 mask, prev = &dn->dn_next; continue; } - fown = &dn->dn_filp->f_owner; + fown = file_f_owner(dn->dn_filp); send_sigio(fown, dn->dn_fd, POLL_MSG); if (dn->dn_mask & FS_DN_MULTISHOT) prev = &dn->dn_next; @@ -316,6 +316,10 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned int arg) goto out_err; } + error = file_f_owner_allocate(filp); + if (error) + goto out_err; + /* set up the new_fsn_mark and new_dn_mark */ new_fsn_mark = &new_dn_mark->fsn_mark; fsnotify_init_mark(new_fsn_mark, dnotify_group); diff --git a/include/linux/fs.h b/include/linux/fs.h index fb0426f349fc5..7af239ca87e29 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -947,6 +947,7 @@ static inline unsigned imajor(const struct inode *inode) } struct fown_struct { + struct file *file; /* backpointer for security modules */ rwlock_t lock; /* protects pid, uid, euid fields */ struct pid *pid; /* pid or -pgrp where SIGIO should be sent */ enum pid_type pid_type; /* Kind of process group SIGIO should be sent to */ @@ -1011,7 +1012,7 @@ struct file { struct mutex f_pos_lock; loff_t f_pos; unsigned int f_flags; - struct fown_struct f_owner; + struct fown_struct *f_owner; const struct cred *f_cred; struct file_ra_state f_ra; struct path f_path; @@ -1076,6 +1077,12 @@ struct file_lease; #define OFFT_OFFSET_MAX type_max(off_t) #endif +int file_f_owner_allocate(struct file *file); +static inline struct fown_struct *file_f_owner(const struct file *file) +{ + return READ_ONCE(file->f_owner); +} + extern void send_sigio(struct fown_struct *fown, int fd, int band); static inline struct inode *file_inode(const struct file *f) @@ -1124,7 +1131,7 @@ extern void __f_setown(struct file *filp, struct pid *, enum pid_type, int force extern int f_setown(struct file *filp, int who, int force); extern void f_delown(struct file *filp); extern pid_t f_getown(struct file *filp); -extern int send_sigurg(struct fown_struct *fown); +extern int send_sigurg(struct file *file); /* * sb->s_flags. Note that these mirror the equivalent MS_* flags where diff --git a/net/core/sock.c b/net/core/sock.c index 9abc4fe259535..bbe4c58470c3d 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -3429,7 +3429,7 @@ static void sock_def_destruct(struct sock *sk) void sk_send_sigurg(struct sock *sk) { if (sk->sk_socket && sk->sk_socket->file) - if (send_sigurg(&sk->sk_socket->file->f_owner)) + if (send_sigurg(sk->sk_socket->file)) sk_wake_async(sk, SOCK_WAKE_URG, POLL_PRI); } EXPORT_SYMBOL(sk_send_sigurg); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index bfa61e005aace..577e1fcbf6dfb 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3950,7 +3950,7 @@ static int selinux_file_send_sigiotask(struct task_struct *tsk, struct file_security_struct *fsec; /* struct fown_struct is never outside the context of a struct file */ - file = container_of(fown, struct file, f_owner); + file = fown->file; fsec = selinux_file(file); diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 4164699cd4f62..cb33920ab67c4 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1950,7 +1950,7 @@ static int smack_file_send_sigiotask(struct task_struct *tsk, /* * struct fown_struct is never outside the context of a struct file */ - file = container_of(fown, struct file, f_owner); + file = fown->file; /* we don't log here as rc can be overriden */ blob = smack_file(file); -- cgit v1.2.3 From 433d7ce2d86d21274838c9e8c796f4232cd13cdb Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Tue, 6 Aug 2024 15:38:12 -0700 Subject: security,bpf: constify struct path in bpf_token_create() LSM hook There is no reason why struct path pointer shouldn't be const-qualified when being passed into bpf_token_create() LSM hook. Add that const. Acked-by: Paul Moore (LSM/SELinux) Suggested-by: Al Viro Signed-off-by: Andrii Nakryiko --- include/linux/lsm_hook_defs.h | 2 +- include/linux/security.h | 4 ++-- security/security.c | 2 +- security/selinux/hooks.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 855db460e08bc..462b553782410 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -431,7 +431,7 @@ LSM_HOOK(int, 0, bpf_prog_load, struct bpf_prog *prog, union bpf_attr *attr, struct bpf_token *token) LSM_HOOK(void, LSM_RET_VOID, bpf_prog_free, struct bpf_prog *prog) LSM_HOOK(int, 0, bpf_token_create, struct bpf_token *token, union bpf_attr *attr, - struct path *path) + const struct path *path) LSM_HOOK(void, LSM_RET_VOID, bpf_token_free, struct bpf_token *token) LSM_HOOK(int, 0, bpf_token_cmd, const struct bpf_token *token, enum bpf_cmd cmd) LSM_HOOK(int, 0, bpf_token_capable, const struct bpf_token *token, int cap) diff --git a/include/linux/security.h b/include/linux/security.h index 1390f1efb4f07..31523a2c71c4d 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -2137,7 +2137,7 @@ extern int security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr, struct bpf_token *token); extern void security_bpf_prog_free(struct bpf_prog *prog); extern int security_bpf_token_create(struct bpf_token *token, union bpf_attr *attr, - struct path *path); + const struct path *path); extern void security_bpf_token_free(struct bpf_token *token); extern int security_bpf_token_cmd(const struct bpf_token *token, enum bpf_cmd cmd); extern int security_bpf_token_capable(const struct bpf_token *token, int cap); @@ -2177,7 +2177,7 @@ static inline void security_bpf_prog_free(struct bpf_prog *prog) { } static inline int security_bpf_token_create(struct bpf_token *token, union bpf_attr *attr, - struct path *path) + const struct path *path) { return 0; } diff --git a/security/security.c b/security/security.c index 8cee5b6c6e6d5..d8d0b67ced250 100644 --- a/security/security.c +++ b/security/security.c @@ -5510,7 +5510,7 @@ int security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr, * Return: Returns 0 on success, error on failure. */ int security_bpf_token_create(struct bpf_token *token, union bpf_attr *attr, - struct path *path) + const struct path *path) { return call_int_hook(bpf_token_create, token, attr, path); } diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 55c78c318ccd7..0eec141a8f37e 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6965,7 +6965,7 @@ static void selinux_bpf_prog_free(struct bpf_prog *prog) } static int selinux_bpf_token_create(struct bpf_token *token, union bpf_attr *attr, - struct path *path) + const struct path *path) { struct bpf_security_struct *bpfsec; -- cgit v1.2.3 From 8a23c9e1ba4642b60420e8caa75859883a509c24 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 19 Sep 2024 11:37:11 -0400 Subject: selinux,smack: properly reference the LSM blob in security_watch_key() Unfortunately when we migrated the lifecycle management of the key LSM blob to the LSM framework we forgot to convert the security_watch_key() callbacks for SELinux and Smack. This patch corrects this by making use of the selinux_key() and smack_key() helper functions respectively. This patch also removes some input checking in the Smack callback as it is no longer needed. Fixes: 5f8d28f6d7d5 ("lsm: infrastructure management of the key security blob") Reported-by: syzbot+044fdf24e96093584232@syzkaller.appspotmail.com Tested-by: syzbot+044fdf24e96093584232@syzkaller.appspotmail.com Reviewed-by: Casey Schaufler Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 +- security/smack/smack_lsm.c | 13 +++---------- 2 files changed, 4 insertions(+), 11 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 81fbfa5b80d4a..67baa487cf7a0 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6720,7 +6720,7 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer) #ifdef CONFIG_KEY_NOTIFICATIONS static int selinux_watch_key(struct key *key) { - struct key_security_struct *ksec = key->security; + struct key_security_struct *ksec = selinux_key(key); u32 sid = current_sid(); return avc_has_perm(sid, ksec->sid, SECCLASS_KEY, KEY__VIEW, NULL); diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index da0c2bffbd085..563fb404f6594 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4629,16 +4629,9 @@ static int smack_watch_key(struct key *key) { struct smk_audit_info ad; struct smack_known *tkp = smk_of_current(); + struct smack_known **blob = smack_key(key); int rc; - if (key == NULL) - return -EINVAL; - /* - * If the key hasn't been initialized give it access so that - * it may do so. - */ - if (key->security == NULL) - return 0; /* * This should not occur */ @@ -4653,8 +4646,8 @@ static int smack_watch_key(struct key *key) ad.a.u.key_struct.key = key->serial; ad.a.u.key_struct.key_desc = key->description; #endif - rc = smk_access(tkp, key->security, MAY_READ, &ad); - rc = smk_bu_note("key watch", tkp, key->security, MAY_READ, rc); + rc = smk_access(tkp, *blob, MAY_READ, &ad); + rc = smk_bu_note("key watch", tkp, *blob, MAY_READ, rc); return rc; } #endif /* CONFIG_KEY_NOTIFICATIONS */ -- cgit v1.2.3