Commit 0188d5c0 authored by Stephen Smalley's avatar Stephen Smalley Committed by Paul Moore
Browse files

selinux: fall back to ref-walk if audit is required 



commit bda0be7a ("security: make inode_follow_link RCU-walk aware")
passed down the rcu flag to the SELinux AVC, but failed to adjust the
test in slow_avc_audit() to also return -ECHILD on LSM_AUDIT_DATA_DENTRY.
Previously, we only returned -ECHILD if generating an audit record with
LSM_AUDIT_DATA_INODE since this was only relevant from inode_permission.
Move the handling of MAY_NOT_BLOCK to avc_audit() and its inlined
equivalent in selinux_inode_permission() immediately after we determine
that audit is required, and always fall back to ref-walk in this case.

Fixes: bda0be7a ("security: make inode_follow_link RCU-walk aware")
Reported-by: default avatarWill Deacon <will@kernel.org>
Suggested-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
Signed-off-by: default avatarStephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
parent 1a37079c

security/selinux/avc.c

+5 −19
Original line number Diff line number Diff line
@@ -424,7 +424,7 @@ static inline int avc_xperms_audit(struct selinux_state *state, 
	if (likely(!audited))

		return 0;

	return slow_avc_audit(state, ssid, tsid, tclass, requested,

			audited, denied, result, ad, 0);

			audited, denied, result, ad);

}



static void avc_node_free(struct rcu_head *rhead)
@@ -758,8 +758,7 @@ static void avc_audit_post_callback(struct audit_buffer *ab, void *a) 
noinline int slow_avc_audit(struct selinux_state *state,

			    u32 ssid, u32 tsid, u16 tclass,

			    u32 requested, u32 audited, u32 denied, int result,

			    struct common_audit_data *a,

			    unsigned int flags)

			    struct common_audit_data *a)

{

	struct common_audit_data stack_data;

	struct selinux_audit_data sad;
@@ -772,17 +771,6 @@ noinline int slow_avc_audit(struct selinux_state *state, 
		a->type = LSM_AUDIT_DATA_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_INODE) &&

	    (flags & MAY_NOT_BLOCK))

		return -ECHILD;



	sad.tclass = tclass;

	sad.requested = requested;

	sad.ssid = ssid;
@@ -855,16 +843,14 @@ static int avc_update_node(struct selinux_avc *avc, 
	/*

	 * If we are in a non-blocking code path, e.g. VFS RCU walk,

	 * then we must not add permissions to a cache entry

	 * because we cannot safely audit the denial.  Otherwise,

	 * because we will not audit the denial.  Otherwise,

	 * during the subsequent blocking retry (e.g. VFS ref walk), we

	 * will find the permissions already granted in the cache entry

	 * and won't audit anything at all, leading to silent denials in

	 * permissive mode that only appear when in enforcing mode.

	 *

	 * See the corresponding handling in slow_avc_audit(), and the

	 * logic in selinux_inode_follow_link and selinux_inode_permission

	 * for the VFS MAY_NOT_BLOCK flag, which is transliterated into

	 * AVC_NONBLOCKING for avc_has_perm_noaudit().

	 * See the corresponding handling of MAY_NOT_BLOCK in avc_audit()

	 * and selinux_inode_permission().

	 */

	if (flags & AVC_NONBLOCKING)

		return 0;

security/selinux/hooks.c

+7 −4
Original line number Diff line number Diff line
@@ -3011,8 +3011,7 @@ static int selinux_inode_follow_link(struct dentry *dentry, struct inode *inode, 


static noinline int audit_inode_permission(struct inode *inode,

					   u32 perms, u32 audited, u32 denied,

					   int result,

					   unsigned flags)

					   int result)

{

	struct common_audit_data ad;

	struct inode_security_struct *isec = selinux_inode(inode);
@@ -3023,7 +3022,7 @@ static noinline int audit_inode_permission(struct inode *inode, 


	rc = slow_avc_audit(&selinux_state,

			    current_sid(), isec->sid, isec->sclass, perms,

			    audited, denied, result, &ad, flags);

			    audited, denied, result, &ad);

	if (rc)

		return rc;

	return 0;
@@ -3070,7 +3069,11 @@ static int selinux_inode_permission(struct inode *inode, int mask) 
	if (likely(!audited))

		return rc;



	rc2 = audit_inode_permission(inode, perms, audited, denied, rc, flags);

	/* fall back to ref-walk if we have to generate audit */

	if (flags & MAY_NOT_BLOCK)

		return -ECHILD;



	rc2 = audit_inode_permission(inode, perms, audited, denied, rc);

	if (rc2)

		return rc2;

	return rc;

security/selinux/include/avc.h

+5 −3
Original line number Diff line number Diff line
@@ -100,8 +100,7 @@ static inline u32 avc_audit_required(u32 requested, 
int slow_avc_audit(struct selinux_state *state,

		   u32 ssid, u32 tsid, u16 tclass,

		   u32 requested, u32 audited, u32 denied, int result,

		   struct common_audit_data *a,

		   unsigned flags);

		   struct common_audit_data *a);



/**

 * avc_audit - Audit the granting or denial of permissions.
@@ -135,9 +134,12 @@ static inline int avc_audit(struct selinux_state *state, 
	audited = avc_audit_required(requested, avd, result, 0, &denied);

	if (likely(!audited))

		return 0;

	/* fall back to ref-walk if we have to generate audit */

	if (flags & MAY_NOT_BLOCK)

		return -ECHILD;

	return slow_avc_audit(state, ssid, tsid, tclass,

			      requested, audited, denied, result,

			      a, flags);

			      a);

}



#define AVC_STRICT 1 /* Ignore permissive mode. */

CRA Git | Maintained and supported by SUSTech CRA and CCSE