selinux: fix avdcache auditing

The per-task avdcache was incorrectly saving and reusing the
audited vector computed by avc_audit_required() rather than
recomputing based on the currently requested permissions and
distinguishing the denied versus allowed cases. As a result,
some permission checks were not being audited, e.g.
directory write checks after a previously cached directory
search check.

Cc: stable@vger.kernel.org
Fixes: dde3a5d0f4dce ("selinux: move avdcache to per-task security struct")
Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
[PM: line wrap tweaks]
Signed-off-by: Paul Moore <paul@paul-moore.com>

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 59942d39ada7f40e651338f8c29b40d6181b8a14..0f704380a8c81177ebf7faceb24df2d74300c8d4 100644 (file)
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3209,15 +3209,13 @@ static inline int task_avdcache_search(struct task_security_struct *tsec,
  * @tsec: the task's security state
  * @isec: the inode associated with the cache entry
  * @avd: the AVD to cache
- * @audited: the permission audit bitmask to cache
  *
- * Update the AVD cache in @tsec with the @avdc and @audited info associated
+ * Update the AVD cache in @tsec with the @avd info associated
  * with @isec.
  */
 static inline void task_avdcache_update(struct task_security_struct *tsec,
                                        struct inode_security_struct *isec,
-                                       struct av_decision *avd,
-                                       u32 audited)
+                                       struct av_decision *avd)
 {
        int spot;
 
@@ -3229,9 +3227,7 @@ static inline void task_avdcache_update(struct task_security_struct *tsec,
        spot = (tsec->avdcache.dir_spot + 1) & (TSEC_AVDC_DIR_SIZE - 1);
        tsec->avdcache.dir_spot = spot;
        tsec->avdcache.dir[spot].isid = isec->sid;
-       tsec->avdcache.dir[spot].audited = audited;
-       tsec->avdcache.dir[spot].allowed = avd->allowed;
-       tsec->avdcache.dir[spot].permissive = avd->flags & AVD_FLAGS_PERMISSIVE;
+       tsec->avdcache.dir[spot].avd = *avd;
        tsec->avdcache.permissive_neveraudit =
                (avd->flags == (AVD_FLAGS_PERMISSIVE|AVD_FLAGS_NEVERAUDIT));
 }
@@ -3252,6 +3248,7 @@ static int selinux_inode_permission(struct inode *inode, int requested)
        struct task_security_struct *tsec;
        struct inode_security_struct *isec;
        struct avdc_entry *avdc;
+       struct av_decision avd, *avdp = &avd;
        int rc, rc2;
        u32 audited, denied;
 
@@ -3273,23 +3270,21 @@ static int selinux_inode_permission(struct inode *inode, int requested)
        rc = task_avdcache_search(tsec, isec, &avdc);
        if (likely(!rc)) {
                /* Cache hit. */
-               audited = perms & avdc->audited;
-               denied = perms & ~avdc->allowed;
-               if (unlikely(denied && enforcing_enabled() &&
-                            !avdc->permissive))
+               avdp = &avdc->avd;
+               denied = perms & ~avdp->allowed;
+               if (unlikely(denied) && enforcing_enabled() &&
+                       !(avdp->flags & AVD_FLAGS_PERMISSIVE))
                        rc = -EACCES;
        } else {
-               struct av_decision avd;
-
                /* Cache miss. */
                rc = avc_has_perm_noaudit(sid, isec->sid, isec->sclass,
-                                         perms, 0, &avd);
-               audited = avc_audit_required(perms, &avd, rc,
-                       (requested & MAY_ACCESS) ? FILE__AUDIT_ACCESS : 0,
-                       &denied);
-               task_avdcache_update(tsec, isec, &avd, audited);
+                                         perms, 0, avdp);
+               task_avdcache_update(tsec, isec, avdp);
        }
 
+       audited = avc_audit_required(perms, avdp, rc,
+                                    (requested & MAY_ACCESS) ?
+                                    FILE__AUDIT_ACCESS : 0, &denied);
        if (likely(!audited))
                return rc;
 

diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index b19e5d978e820137c0c5c6cb240f1fb2d8d6be03..3c0a16ec978b01620ef71f99a92c8a9a813f0948 100644 (file)
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -32,9 +32,7 @@
 
 struct avdc_entry {
        u32 isid; /* inode SID */
-       u32 allowed; /* allowed permission bitmask */
-       u32 audited; /* audited permission bitmask */
-       bool permissive; /* AVC permissive flag */
+       struct av_decision avd; /* av decision */
 };
 
 struct cred_security_struct {