apparmor: fix fast path cache check for unix sockets

The fast path cache check is incorrect forcing more slow path
revalidations than necessary, because the unix logic check is inverted.

Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>

diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index c751f2774c59fda44b1e4a44516d654a29938377..694e157149e85e47961f8217f8dc8afc80db16e3 100644 (file)
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -572,8 +572,7 @@ static bool __file_is_delegated(struct aa_label *obj_label)
        return unconfined(obj_label);
 }
 
-static bool __unix_needs_revalidation(struct file *file, struct aa_label *label,
-                                     u32 request)
+static bool __is_unix_file(struct file *file)
 {
        struct socket *sock = (struct socket *) file->private_data;
 
@@ -581,23 +580,31 @@ static bool __unix_needs_revalidation(struct file *file, struct aa_label *label,
 
        if (!S_ISSOCK(file_inode(file)->i_mode))
                return false;
-       if (request & NET_PEER_MASK)
-               return false;
        /* sock and sock->sk can be NULL for sockets being set up or torn down */
        if (!sock || !sock->sk)
                return false;
-       if (sock->sk->sk_family == PF_UNIX) {
-               struct aa_sk_ctx *ctx = aa_sock(sock->sk);
-
-               if (rcu_access_pointer(ctx->peer) !=
-                   rcu_access_pointer(ctx->peer_lastupdate))
-                       return true;
-               return !__aa_subj_label_is_cached(rcu_dereference(ctx->label),
-                                                 label);
-       }
+       if (sock->sk->sk_family == PF_UNIX)
+               return true;
        return false;
 }
 
+static bool __unix_needs_revalidation(struct file *file, struct aa_label *label,
+                                     u32 request)
+{
+       struct socket *sock = (struct socket *) file->private_data;
+
+       AA_BUG(!__is_unix_file(file));
+       lockdep_assert_in_rcu_read_lock();
+
+       struct aa_sk_ctx *skctx = aa_sock(sock->sk);
+
+       if (rcu_access_pointer(skctx->peer) !=
+           rcu_access_pointer(skctx->peer_lastupdate))
+               return true;
+
+       return !__aa_subj_label_is_cached(rcu_dereference(skctx->label), label);
+}
+
 /**
  * aa_file_perm - do permission revalidation check & audit for @file
  * @op: operation being checked
@@ -640,7 +647,7 @@ int aa_file_perm(const char *op, const struct cred *subj_cred,
         */
        denied = request & ~fctx->allow;
        if (unconfined(label) || __file_is_delegated(flabel) ||
-           __unix_needs_revalidation(file, label, request) ||
+           (!denied && __is_unix_file(file) && !__unix_needs_revalidation(file, label, request)) ||
            (!denied && __aa_subj_label_is_cached(label, flabel))) {
                rcu_read_unlock();
                goto done;