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

commit 0188d5c025ca8fe756ba3193bd7d150139af5a88 upstream.

commit bda0be7ad994 ("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: bda0be7ad994 ("security: make inode_follow_link RCU-walk aware")
Reported-by: Will Deacon <will@kernel.org>
Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index 74c43ebe34bb818beac3279110f796a7ce8ba97b..23dc888ae3056c6ec5fc2cceb36c70f327ca737d 100644 (file)
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -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;

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 0426ce3c19e907b53ad87fb0936c9603b2cbf22b..39410913a694904261a17bf5c2a00f46edbd14e1 100644 (file)
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3023,8 +3023,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);
@@ -3035,7 +3034,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;
@@ -3082,7 +3081,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;

diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h
index 74ea50977c201fb815431fb356e0858e4239881c..cf4cc3ef959b5b88c13b785e54394a0d45765610 100644 (file)
--- a/security/selinux/include/avc.h
+++ b/security/selinux/include/avc.h
@@ -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. */