SELINUX: Make selinux cache VFS RCU walks safe

Now that the security modules can decide whether they support the dcache RCU walk or not it's possible to make selinux a bit more RCU friendly. The SELinux AVC and security server access decision code is RCU safe. A specific piece of the LSM audit code may not be RCU safe. This patch makes the VFS RCU walk retry if it would hit the non RCU safe chunk of code. It will normally just work under RCU. This is done simply by passing the VFS RCU state as a flag down into the avc_audit() code and returning ECHILD there if it would have an issue. Based-on-patch-by: Andi Kleen <ak@linux.intel.com> Signed-off-by: Eric Paris <eparis@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

SELINUX: Make selinux cache VFS RCU walks safe
Now that the security modules can decide whether they support the dcache RCU walk or not it's possible to make selinux a bit more RCU friendly. The SELinux AVC and security server access decision code is RCU safe. A specific piece of the LSM audit code may not be RCU safe. This patch makes the VFS RCU walk retry if it would hit the non RCU safe chunk of code. It will normally just work under RCU. This is done simply by passing the VFS RCU state as a flag down into the avc_audit() code and returning ECHILD there if it would have an issue. Based-on-patch-by: Andi Kleen <ak@linux.intel.com> Signed-off-by: Eric Paris <eparis@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Eric Paris · Linus Torvalds
1 parent 1879fd6a26
Showing 3 changed files with 55 additions and 25 deletions Side-by-side Diff
security/selinux/avc.c
security/selinux/hooks.c
security/selinux/include/avc.h
@@ -471,6 +471,7 @@
  * @avd: access vector decisions
  * @result: result from avc_has_perm_noaudit
  * @a:  auxiliary audit data
+ * @flags: VFS walk flags
  *
  * Audit the granting or denial of permissions in accordance
  * with the policy.  This function is typically called by
  
@@ -481,9 +482,10 @@
  * be performed under a lock, to allow the lock to be released
  * before calling the auditing code.
  */
-void avc_audit(u32 ssid, u32 tsid,
+int avc_audit(u32 ssid, u32 tsid,
 	       u16 tclass, u32 requested,
-	       struct av_decision *avd, int result, struct common_audit_data *a)
+	       struct av_decision *avd, int result, struct common_audit_data *a,
+	       unsigned flags)
 {
 	struct common_audit_data stack_data;
 	u32 denied, audited;
  
@@ -515,11 +517,24 @@
 	else
 		audited = requested & avd->auditallow;
 	if (!audited)
-		return;
+		return 0;
+
 	if (!a) {
 		a = &stack_data;
 		COMMON_AUDIT_DATA_INIT(a, NONE);
 	}
+
+	/*
+	 * When in a RCU walk do the audit on the RCU retry.  This is because
+	 * the collection of the dname in an inode audit message is not RCU
+	 * safe.  Note this may drop some audits when the situation changes
+	 * during retry. However this is logically just as if the operation
+	 * happened a little later.
+	 */
+	if ((a->type == LSM_AUDIT_DATA_FS) &&
+	    (flags & IPERM_FLAG_RCU))
+		return -ECHILD;
+
 	a->selinux_audit_data.tclass = tclass;
 	a->selinux_audit_data.requested = requested;
 	a->selinux_audit_data.ssid = ssid;
@@ -529,6 +544,7 @@
 	a->lsm_pre_audit = avc_audit_pre_callback;
 	a->lsm_post_audit = avc_audit_post_callback;
 	common_lsm_audit(a);
+	return 0;
 }
  
 /**
@@ -793,6 +809,7 @@
  * @tclass: target security class
  * @requested: requested permissions, interpreted based on @tclass
  * @auditdata: auxiliary audit data
+ * @flags: VFS walk flags
  *
  * Check the AVC to determine whether the @requested permissions are granted
  * for the SID pair (@ssid, @tsid), interpreting the permissions
  
  
@@ -802,14 +819,19 @@
  * permissions are granted, -%EACCES if any permissions are denied, or
  * another -errno upon other errors.
  */
-int avc_has_perm(u32 ssid, u32 tsid, u16 tclass,
-		 u32 requested, struct common_audit_data *auditdata)
+int avc_has_perm_flags(u32 ssid, u32 tsid, u16 tclass,
+		       u32 requested, struct common_audit_data *auditdata,
+		       unsigned flags)
 {
 	struct av_decision avd;
-	int rc;
+	int rc, rc2;
  
 	rc = avc_has_perm_noaudit(ssid, tsid, tclass, requested, 0, &avd);
-	avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata);
+
+	rc2 = avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata,
+			flags);
+	if (rc2)
+		return rc2;
 	return rc;
 }
  
@@ -1446,8 +1446,11 @@
 	}
  
 	rc = avc_has_perm_noaudit(sid, sid, sclass, av, 0, &avd);
-	if (audit == SECURITY_CAP_AUDIT)
-		avc_audit(sid, sid, sclass, av, &avd, rc, &ad);
+	if (audit == SECURITY_CAP_AUDIT) {
+		int rc2 = avc_audit(sid, sid, sclass, av, &avd, rc, &ad, 0);
+		if (rc2)
+			return rc2;
+	}
 	return rc;
 }
  
@@ -1467,7 +1470,8 @@
 static int inode_has_perm(const struct cred *cred,
 			  struct inode *inode,
 			  u32 perms,
-			  struct common_audit_data *adp)
+			  struct common_audit_data *adp,
+			  unsigned flags)
 {
 	struct inode_security_struct *isec;
 	struct common_audit_data ad;
@@ -1487,7 +1491,7 @@
 		ad.u.fs.inode = inode;
 	}
  
-	return avc_has_perm(sid, isec->sid, isec->sclass, perms, adp);
+	return avc_has_perm_flags(sid, isec->sid, isec->sclass, perms, adp, flags);
 }
  
 /* Same as inode_has_perm, but pass explicit audit data containing
@@ -1504,7 +1508,7 @@
 	COMMON_AUDIT_DATA_INIT(&ad, FS);
 	ad.u.fs.path.mnt = mnt;
 	ad.u.fs.path.dentry = dentry;
-	return inode_has_perm(cred, inode, av, &ad);
+	return inode_has_perm(cred, inode, av, &ad, 0);
 }
  
 /* Check whether a task can use an open file descriptor to
@@ -1540,7 +1544,7 @@
 	/* av is zero if only checking access to the descriptor. */
 	rc = 0;
 	if (av)
-		rc = inode_has_perm(cred, inode, av, &ad);
+		rc = inode_has_perm(cred, inode, av, &ad, 0);
  
 out:
 	return rc;
@@ -2103,7 +2107,7 @@
 			file = file_priv->file;
 			inode = file->f_path.dentry->d_inode;
 			if (inode_has_perm(cred, inode,
-					   FILE__READ | FILE__WRITE, NULL)) {
+					   FILE__READ | FILE__WRITE, NULL, 0)) {
 				drop_tty = 1;
 			}
 		}
@@ -2649,10 +2653,6 @@
 	if (!mask)
 		return 0;
  
-	/* May be droppable after audit */
-	if (flags & IPERM_FLAG_RCU)
-		return -ECHILD;
-
 	COMMON_AUDIT_DATA_INIT(&ad, FS);
 	ad.u.fs.inode = inode;
  
@@ -2661,7 +2661,7 @@
  
 	perms = file_mask_to_av(inode->i_mode, mask);
  
-	return inode_has_perm(cred, inode, perms, &ad);
+	return inode_has_perm(cred, inode, perms, &ad, flags);
 }
  
 static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr)
@@ -3209,7 +3209,7 @@
 	 * new inode label or new policy.
 	 * This check is not redundant - do not remove.
 	 */
-	return inode_has_perm(cred, inode, open_file_to_av(file), NULL);
+	return inode_has_perm(cred, inode, open_file_to_av(file), NULL, 0);
 }
  
 /* task security operations */
@@ -54,11 +54,11 @@
  
 void __init avc_init(void);
  
-void avc_audit(u32 ssid, u32 tsid,
+int avc_audit(u32 ssid, u32 tsid,
 	       u16 tclass, u32 requested,
 	       struct av_decision *avd,
 	       int result,
-	       struct common_audit_data *a);
+	      struct common_audit_data *a, unsigned flags);
  
 #define AVC_STRICT 1 /* Ignore permissive mode. */
 int avc_has_perm_noaudit(u32 ssid, u32 tsid,
@@ -66,9 +66,17 @@
 			 unsigned flags,
 			 struct av_decision *avd);
  
-int avc_has_perm(u32 ssid, u32 tsid,
-		 u16 tclass, u32 requested,
-		 struct common_audit_data *auditdata);
+int avc_has_perm_flags(u32 ssid, u32 tsid,
+		       u16 tclass, u32 requested,
+		       struct common_audit_data *auditdata,
+		       unsigned);
+
+static inline int avc_has_perm(u32 ssid, u32 tsid,
+			       u16 tclass, u32 requested,
+			       struct common_audit_data *auditdata)
+{
+	return avc_has_perm_flags(ssid, tsid, tclass, requested, auditdata, 0);
+}
  
 u32 avc_policy_seqno(void);
...	...	@@ -471,6 +471,7 @@
471	471	* @avd: access vector decisions
472	472	* @result: result from avc_has_perm_noaudit
473	473	* @a: auxiliary audit data
	474	+ * @flags: VFS walk flags
474	475	*
475	476	* Audit the granting or denial of permissions in accordance
476	477	* with the policy. This function is typically called by
477	478
...	...	@@ -481,9 +482,10 @@
481	482	* be performed under a lock, to allow the lock to be released
482	483	* before calling the auditing code.
483	484	*/
484		-void avc_audit(u32 ssid, u32 tsid,
	485	+int avc_audit(u32 ssid, u32 tsid,
485	486	u16 tclass, u32 requested,
486		- struct av_decision avd, int result, struct common_audit_data a)
	487	+ struct av_decision avd, int result, struct common_audit_data a,
	488	+ unsigned flags)
487	489	{
488	490	struct common_audit_data stack_data;
489	491	u32 denied, audited;
490	492
...	...	@@ -515,11 +517,24 @@
515	517	else
516	518	audited = requested & avd->auditallow;
517	519	if (!audited)
518		- return;
	520	+ return 0;
	521	+
519	522	if (!a) {
520	523	a = &stack_data;
521	524	COMMON_AUDIT_DATA_INIT(a, NONE);
522	525	}
	526	+
	527	+ /*
	528	+ * When in a RCU walk do the audit on the RCU retry. This is because
	529	+ * the collection of the dname in an inode audit message is not RCU
	530	+ * safe. Note this may drop some audits when the situation changes
	531	+ * during retry. However this is logically just as if the operation
	532	+ * happened a little later.
	533	+ */
	534	+ if ((a->type == LSM_AUDIT_DATA_FS) &&
	535	+ (flags & IPERM_FLAG_RCU))
	536	+ return -ECHILD;
	537	+
523	538	a->selinux_audit_data.tclass = tclass;
524	539	a->selinux_audit_data.requested = requested;
525	540	a->selinux_audit_data.ssid = ssid;
...	...	@@ -529,6 +544,7 @@
529	544	a->lsm_pre_audit = avc_audit_pre_callback;
530	545	a->lsm_post_audit = avc_audit_post_callback;
531	546	common_lsm_audit(a);
	547	+ return 0;
532	548	}
533	549
534	550	/**
...	...	@@ -793,6 +809,7 @@
793	809	* @tclass: target security class
794	810	* @requested: requested permissions, interpreted based on @tclass
795	811	* @auditdata: auxiliary audit data
	812	+ * @flags: VFS walk flags
796	813	*
797	814	* Check the AVC to determine whether the @requested permissions are granted
798	815	* for the SID pair (@ssid, @tsid), interpreting the permissions
799	816
800	817
...	...	@@ -802,14 +819,19 @@
802	819	* permissions are granted, -%EACCES if any permissions are denied, or
803	820	* another -errno upon other errors.
804	821	*/
805		-int avc_has_perm(u32 ssid, u32 tsid, u16 tclass,
806		- u32 requested, struct common_audit_data *auditdata)
	822	+int avc_has_perm_flags(u32 ssid, u32 tsid, u16 tclass,
	823	+ u32 requested, struct common_audit_data *auditdata,
	824	+ unsigned flags)
807	825	{
808	826	struct av_decision avd;
809		- int rc;
	827	+ int rc, rc2;
810	828
811	829	rc = avc_has_perm_noaudit(ssid, tsid, tclass, requested, 0, &avd);
812		- avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata);
	830	+
	831	+ rc2 = avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata,
	832	+ flags);
	833	+ if (rc2)
	834	+ return rc2;
813	835	return rc;
814	836	}
815	837
...	...	@@ -1446,8 +1446,11 @@
1446	1446	}
1447	1447
1448	1448	rc = avc_has_perm_noaudit(sid, sid, sclass, av, 0, &avd);
1449		- if (audit == SECURITY_CAP_AUDIT)
1450		- avc_audit(sid, sid, sclass, av, &avd, rc, &ad);
	1449	+ if (audit == SECURITY_CAP_AUDIT) {
	1450	+ int rc2 = avc_audit(sid, sid, sclass, av, &avd, rc, &ad, 0);
	1451	+ if (rc2)
	1452	+ return rc2;
	1453	+ }
1451	1454	return rc;
1452	1455	}
1453	1456
...	...	@@ -1467,7 +1470,8 @@
1467	1470	static int inode_has_perm(const struct cred *cred,
1468	1471	struct inode *inode,
1469	1472	u32 perms,
1470		- struct common_audit_data *adp)
	1473	+ struct common_audit_data *adp,
	1474	+ unsigned flags)
1471	1475	{
1472	1476	struct inode_security_struct *isec;
1473	1477	struct common_audit_data ad;
...	...	@@ -1487,7 +1491,7 @@
1487	1491	ad.u.fs.inode = inode;
1488	1492	}
1489	1493
1490		- return avc_has_perm(sid, isec->sid, isec->sclass, perms, adp);
	1494	+ return avc_has_perm_flags(sid, isec->sid, isec->sclass, perms, adp, flags);
1491	1495	}
1492	1496
1493	1497	/* Same as inode_has_perm, but pass explicit audit data containing
...	...	@@ -1504,7 +1508,7 @@
1504	1508	COMMON_AUDIT_DATA_INIT(&ad, FS);
1505	1509	ad.u.fs.path.mnt = mnt;
1506	1510	ad.u.fs.path.dentry = dentry;
1507		- return inode_has_perm(cred, inode, av, &ad);
	1511	+ return inode_has_perm(cred, inode, av, &ad, 0);
1508	1512	}
1509	1513
1510	1514	/* Check whether a task can use an open file descriptor to
...	...	@@ -1540,7 +1544,7 @@
1540	1544	/* av is zero if only checking access to the descriptor. */
1541	1545	rc = 0;
1542	1546	if (av)
1543		- rc = inode_has_perm(cred, inode, av, &ad);
	1547	+ rc = inode_has_perm(cred, inode, av, &ad, 0);
1544	1548
1545	1549	out:
1546	1550	return rc;
...	...	@@ -2103,7 +2107,7 @@
2103	2107	file = file_priv->file;
2104	2108	inode = file->f_path.dentry->d_inode;
2105	2109	if (inode_has_perm(cred, inode,
2106		- FILE__READ \| FILE__WRITE, NULL)) {
	2110	+ FILE__READ \| FILE__WRITE, NULL, 0)) {
2107	2111	drop_tty = 1;
2108	2112	}
2109	2113	}
...	...	@@ -2649,10 +2653,6 @@
2649	2653	if (!mask)
2650	2654	return 0;
2651	2655
2652		- /* May be droppable after audit */
2653		- if (flags & IPERM_FLAG_RCU)
2654		- return -ECHILD;
2655		-
2656	2656	COMMON_AUDIT_DATA_INIT(&ad, FS);
2657	2657	ad.u.fs.inode = inode;
2658	2658
...	...	@@ -2661,7 +2661,7 @@
2661	2661
2662	2662	perms = file_mask_to_av(inode->i_mode, mask);
2663	2663
2664		- return inode_has_perm(cred, inode, perms, &ad);
	2664	+ return inode_has_perm(cred, inode, perms, &ad, flags);
2665	2665	}
2666	2666
2667	2667	static int selinux_inode_setattr(struct dentry dentry, struct iattr iattr)
...	...	@@ -3209,7 +3209,7 @@
3209	3209	* new inode label or new policy.
3210	3210	* This check is not redundant - do not remove.
3211	3211	*/
3212		- return inode_has_perm(cred, inode, open_file_to_av(file), NULL);
	3212	+ return inode_has_perm(cred, inode, open_file_to_av(file), NULL, 0);
3213	3213	}
3214	3214
3215	3215	/* task security operations */
...	...	@@ -54,11 +54,11 @@
54	54
55	55	void __init avc_init(void);
56	56
57		-void avc_audit(u32 ssid, u32 tsid,
	57	+int avc_audit(u32 ssid, u32 tsid,
58	58	u16 tclass, u32 requested,
59	59	struct av_decision *avd,
60	60	int result,
61		- struct common_audit_data *a);
	61	+ struct common_audit_data *a, unsigned flags);
62	62
63	63	#define AVC_STRICT 1 /* Ignore permissive mode. */
64	64	int avc_has_perm_noaudit(u32 ssid, u32 tsid,
...	...	@@ -66,9 +66,17 @@
66	66	unsigned flags,
67	67	struct av_decision *avd);
68	68
69		-int avc_has_perm(u32 ssid, u32 tsid,
70		- u16 tclass, u32 requested,
71		- struct common_audit_data *auditdata);
	69	+int avc_has_perm_flags(u32 ssid, u32 tsid,
	70	+ u16 tclass, u32 requested,
	71	+ struct common_audit_data *auditdata,
	72	+ unsigned);
	73	+
	74	+static inline int avc_has_perm(u32 ssid, u32 tsid,
	75	+ u16 tclass, u32 requested,
	76	+ struct common_audit_data *auditdata)
	77	+{
	78	+ return avc_has_perm_flags(ssid, tsid, tclass, requested, auditdata, 0);
	79	+}
72	80
73	81	u32 avc_policy_seqno(void);
74	82