Merge tag 'selinux-pr-20240911' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux

Pull selinux updates from Paul Moore:

 - Ensure that both IPv4 and IPv6 connections are properly initialized

   While we always properly initialized IPv4 connections early in their
   life, we missed the necessary IPv6 change when we were adding IPv6
   support.

 - Annotate the SELinux inode revalidation function to quiet KCSAN

   KCSAN correctly identifies a race in __inode_security_revalidate()
   when we check to see if an inode's SELinux has been properly
   initialized. While KCSAN is correct, it is an intentional choice made
   for performance reasons; if necessary, we check the state a second
   time, this time with a lock held, before initializing the inode's
   state.

 - Code cleanups, simplification, etc.

   A handful of individual patches to simplify some SELinux kernel
   logic, improve return code granularity via ERR_PTR(), follow the
   guidance on using KMEM_CACHE(), and correct some minor style
   problems.

* tag 'selinux-pr-20240911' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux:
  selinux: fix style problems in security/selinux/include/audit.h
  selinux: simplify avc_xperms_audit_required()
  selinux: mark both IPv4 and IPv6 accepted connection sockets as labeled
  selinux: replace kmem_cache_create() with KMEM_CACHE()
  selinux: annotate false positive data race to avoid KCSAN warnings
  selinux: refactor code to return ERR_PTR in selinux_netlbl_sock_genattr
  selinux: Streamline type determination in security_compute_sid

8 files changed, 68 insertions, 76 deletions

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index b49c44869dc4..cc0b0af20296 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -134,18 +134,10 @@ static inline u32 avc_hash(u32 ssid, u32 tsid, u16 tclass)
  */
 void __init avc_init(void)
 {
-	avc_node_cachep = kmem_cache_create("avc_node", sizeof(struct avc_node),
-					0, SLAB_PANIC, NULL);
-	avc_xperms_cachep = kmem_cache_create("avc_xperms_node",
-					sizeof(struct avc_xperms_node),
-					0, SLAB_PANIC, NULL);
-	avc_xperms_decision_cachep = kmem_cache_create(
-					"avc_xperms_decision_node",
-					sizeof(struct avc_xperms_decision_node),
-					0, SLAB_PANIC, NULL);
-	avc_xperms_data_cachep = kmem_cache_create("avc_xperms_data",
-					sizeof(struct extended_perms_data),
-					0, SLAB_PANIC, NULL);
+	avc_node_cachep = KMEM_CACHE(avc_node, SLAB_PANIC);
+	avc_xperms_cachep = KMEM_CACHE(avc_xperms_node, SLAB_PANIC);
+	avc_xperms_decision_cachep = KMEM_CACHE(avc_xperms_decision_node, SLAB_PANIC);
+	avc_xperms_data_cachep = KMEM_CACHE(extended_perms_data, SLAB_PANIC);
 }
 
 int avc_get_hash_stats(char *page)
@@ -396,7 +388,7 @@ static inline u32 avc_xperms_audit_required(u32 requested,
 		audited = denied & avd->auditdeny;
 		if (audited && xpd) {
 			if (avc_xperms_has_perm(xpd, perm, XPERMS_DONTAUDIT))
-				audited &= ~requested;
+				audited = 0;
 		}
 	} else if (result) {
 		audited = denied = requested;
@@ -404,7 +396,7 @@ static inline u32 avc_xperms_audit_required(u32 requested,
 		audited = requested & avd->auditallow;
 		if (audited && xpd) {
 			if (!avc_xperms_has_perm(xpd, perm, XPERMS_AUDITALLOW))
-				audited &= ~requested;
+				audited = 0;
 		}
 	}
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index cc123c6390a7..e9b630de2f79 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;
 
diff --git a/security/selinux/include/audit.h b/security/selinux/include/audit.h
index 29c7d4c86f6d..168d17be7df3 100644
--- a/security/selinux/include/audit.h
+++ b/security/selinux/include/audit.h
@@ -16,45 +16,45 @@
 #include <linux/types.h>
 
 /**
- *	selinux_audit_rule_init - alloc/init an selinux audit rule structure.
- *	@field: the field this rule refers to
- *	@op: the operator the rule uses
- *	@rulestr: the text "target" of the rule
- *	@rule: pointer to the new rule structure returned via this
- *	@gfp: GFP flag used for kmalloc
+ * selinux_audit_rule_init - alloc/init an selinux audit rule structure.
+ * @field: the field this rule refers to
+ * @op: the operator the rule uses
+ * @rulestr: the text "target" of the rule
+ * @rule: pointer to the new rule structure returned via this
+ * @gfp: GFP flag used for kmalloc
  *
- *	Returns 0 if successful, -errno if not.  On success, the rule structure
- *	will be allocated internally.  The caller must free this structure with
- *	selinux_audit_rule_free() after use.
+ * Returns 0 if successful, -errno if not.  On success, the rule structure
+ * will be allocated internally.  The caller must free this structure with
+ * selinux_audit_rule_free() after use.
  */
 int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **rule,
 			    gfp_t gfp);
 
 /**
- *	selinux_audit_rule_free - free an selinux audit rule structure.
- *	@rule: pointer to the audit rule to be freed
+ * selinux_audit_rule_free - free an selinux audit rule structure.
+ * @rule: pointer to the audit rule to be freed
  *
- *	This will free all memory associated with the given rule.
- *	If @rule is NULL, no operation is performed.
+ * This will free all memory associated with the given rule.
+ * If @rule is NULL, no operation is performed.
  */
 void selinux_audit_rule_free(void *rule);
 
 /**
- *	selinux_audit_rule_match - determine if a context ID matches a rule.
- *	@sid: the context ID to check
- *	@field: the field this rule refers to
- *	@op: the operator the rule uses
- *	@rule: pointer to the audit rule to check against
+ * selinux_audit_rule_match - determine if a context ID matches a rule.
+ * @sid: the context ID to check
+ * @field: the field this rule refers to
+ * @op: the operator the rule uses
+ * @rule: pointer to the audit rule to check against
  *
- *	Returns 1 if the context id matches the rule, 0 if it does not, and
- *	-errno on failure.
+ * Returns 1 if the context id matches the rule, 0 if it does not, and
+ * -errno on failure.
  */
 int selinux_audit_rule_match(u32 sid, u32 field, u32 op, void *rule);
 
 /**
- *	selinux_audit_rule_known - check to see if rule contains selinux fields.
- *	@rule: rule to be checked
- *	Returns 1 if there are selinux fields specified in the rule, 0 otherwise.
+ * selinux_audit_rule_known - check to see if rule contains selinux fields.
+ * @rule: rule to be checked
+ * Returns 1 if there are selinux fields specified in the rule, 0 otherwise.
  */
 int selinux_audit_rule_known(struct audit_krule *rule);
 
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index 55885634e880..5ad2fd68abbf 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -62,7 +62,7 @@ static int selinux_netlbl_sidlookup_cached(struct sk_buff *skb,
  * Description:
  * Generate the NetLabel security attributes for a socket, making full use of
  * the socket's attribute cache.  Returns a pointer to the security attributes
- * on success, NULL on failure.
+ * on success, or an ERR_PTR on failure.
  *
  */
 static struct netlbl_lsm_secattr *selinux_netlbl_sock_genattr(struct sock *sk)
@@ -76,11 +76,12 @@ static struct netlbl_lsm_secattr *selinux_netlbl_sock_genattr(struct sock *sk)
 
 	secattr = netlbl_secattr_alloc(GFP_ATOMIC);
 	if (secattr == NULL)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
+
 	rc = security_netlbl_sid_to_secattr(sksec->sid, secattr);
 	if (rc != 0) {
 		netlbl_secattr_free(secattr);
-		return NULL;
+		return ERR_PTR(rc);
 	}
 	sksec->nlbl_secattr = secattr;
 
@@ -358,7 +359,7 @@ void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family)
 {
 	struct sk_security_struct *sksec = sk->sk_security;
 
-	if (family == PF_INET)
+	if (family == PF_INET || family == PF_INET6)
 		sksec->nlbl_state = NLBL_LABELED;
 	else
 		sksec->nlbl_state = NLBL_UNSET;
@@ -400,8 +401,8 @@ int selinux_netlbl_socket_post_create(struct sock *sk, u16 family)
 		return 0;
 
 	secattr = selinux_netlbl_sock_genattr(sk);
-	if (secattr == NULL)
-		return -ENOMEM;
+	if (IS_ERR(secattr))
+		return PTR_ERR(secattr);
 	/* On socket creation, replacement of IP options is safe even if
 	 * the caller does not hold the socket lock.
 	 */
@@ -561,10 +562,9 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
 		return rc;
 	}
 	secattr = selinux_netlbl_sock_genattr(sk);
-	if (secattr == NULL) {
-		rc = -ENOMEM;
-		return rc;
-	}
+	if (IS_ERR(secattr))
+		return PTR_ERR(secattr);
+
 	rc = netlbl_conn_setattr(sk, addr, secattr);
 	if (rc == 0)
 		sksec->nlbl_state = NLBL_CONNLABELED;
diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index 2ad98732d052..8e400dd736b7 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -604,9 +604,6 @@ int avtab_write(struct policydb *p, struct avtab *a, void *fp)
 
 void __init avtab_cache_init(void)
 {
-	avtab_node_cachep = kmem_cache_create(
-		"avtab_node", sizeof(struct avtab_node), 0, SLAB_PANIC, NULL);
-	avtab_xperms_cachep = kmem_cache_create(
-		"avtab_extended_perms", sizeof(struct avtab_extended_perms), 0,
-		SLAB_PANIC, NULL);
+	avtab_node_cachep = KMEM_CACHE(avtab_node, SLAB_PANIC);
+	avtab_xperms_cachep = KMEM_CACHE(avtab_extended_perms, SLAB_PANIC);
 }
diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
index 04d7f4907a06..99c01be15115 100644
--- a/security/selinux/ss/ebitmap.c
+++ b/security/selinux/ss/ebitmap.c
@@ -572,7 +572,5 @@ u32 ebitmap_hash(const struct ebitmap *e, u32 hash)
 
 void __init ebitmap_cache_init(void)
 {
-	ebitmap_node_cachep = kmem_cache_create("ebitmap_node",
-						sizeof(struct ebitmap_node), 0,
-						SLAB_PANIC, NULL);
+	ebitmap_node_cachep = KMEM_CACHE(ebitmap_node, SLAB_PANIC);
 }
diff --git a/security/selinux/ss/hashtab.c b/security/selinux/ss/hashtab.c
index 32c4cb37f3d2..383fd2d70878 100644
--- a/security/selinux/ss/hashtab.c
+++ b/security/selinux/ss/hashtab.c
@@ -194,7 +194,5 @@ error:
 
 void __init hashtab_cache_init(void)
 {
-	hashtab_node_cachep = kmem_cache_create("hashtab_node",
-						sizeof(struct hashtab_node), 0,
-						SLAB_PANIC, NULL);
+	hashtab_node_cachep = KMEM_CACHE(hashtab_node, SLAB_PANIC);
 }
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index e33e55384b75..a9830fbfc5c6 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1804,22 +1804,9 @@ retry:
 			newcontext.role = OBJECT_R_VAL;
 	}
 
-	/* Set the type to default values. */
-	if (cladatum && cladatum->default_type == DEFAULT_SOURCE) {
-		newcontext.type = scontext->type;
-	} else if (cladatum && cladatum->default_type == DEFAULT_TARGET) {
-		newcontext.type = tcontext->type;
-	} else {
-		if ((tclass == policydb->process_class) || sock) {
-			/* Use the type of process. */
-			newcontext.type = scontext->type;
-		} else {
-			/* Use the type of the related object. */
-			newcontext.type = tcontext->type;
-		}
-	}
-
-	/* Look for a type transition/member/change rule. */
+	/* Set the type.
+	 * Look for a type transition/member/change rule.
+	 */
 	avkey.source_type = scontext->type;
 	avkey.target_type = tcontext->type;
 	avkey.target_class = tclass;
@@ -1837,9 +1824,24 @@ retry:
 		}
 	}
 
+	/* If a permanent rule is found, use the type from
+	 * the type transition/member/change rule. Otherwise,
+	 * set the type to its default values.
+	 */
 	if (avnode) {
-		/* Use the type from the type transition/member/change rule. */
 		newcontext.type = avnode->datum.u.data;
+	} else if (cladatum && cladatum->default_type == DEFAULT_SOURCE) {
+		newcontext.type = scontext->type;
+	} else if (cladatum && cladatum->default_type == DEFAULT_TARGET) {
+		newcontext.type = tcontext->type;
+	} else {
+		if ((tclass == policydb->process_class) || sock) {
+			/* Use the type of process. */
+			newcontext.type = scontext->type;
+		} else {
+			/* Use the type of the related object. */
+			newcontext.type = tcontext->type;
+		}
 	}
 
 	/* if we have a objname this is a file trans check so check those rules */