apparmor: move aa_file_perm() to use labels

Signed-off-by: John Johansen <john.johansen@canonical.com>

diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index 5289c8db832bcfeda912032f581337c7bfd18696..c13e967137a8a4ff2c29e1ddda2c6202759aa369 100644 (file)
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -23,6 +23,7 @@
 #include "include/match.h"
 #include "include/path.h"
 #include "include/policy.h"
+#include "include/label.h"
 
 static u32 map_mask_to_chr_mask(u32 mask)
 {
@@ -433,22 +434,55 @@ audit:
 /**
  * aa_file_perm - do permission revalidation check & audit for @file
  * @op: operation being checked
- * @profile: profile being enforced   (NOT NULL)
+ * @label: label being enforced   (NOT NULL)
  * @file: file to revalidate access permissions on  (NOT NULL)
  * @request: requested permissions
  *
  * Returns: %0 if access allowed else error
  */
-int aa_file_perm(const char *op, struct aa_profile *profile, struct file *file,
+int aa_file_perm(const char *op, struct aa_label *label, struct file *file,
                 u32 request)
 {
        struct path_cond cond = {
                .uid = file_inode(file)->i_uid,
                .mode = file_inode(file)->i_mode
        };
+       struct aa_file_ctx *fctx;
+       struct aa_label *flabel;
+       u32 denied;
+       int error = 0;
+
+       AA_BUG(!label);
+       AA_BUG(!file);
+
+       fctx = file_ctx(file);
+
+       rcu_read_lock();
+       flabel  = rcu_dereference(fctx->label);
+       AA_BUG(!flabel);
+
+       /* revalidate access, if task is unconfined, or the cached cred
+        * doesn't match or if the request is for more permissions than
+        * was granted.
+        *
+        * Note: the test for !unconfined(flabel) is to handle file
+        *       delegation from unconfined tasks
+        */
+       denied = request & ~fctx->allow;
+       if (unconfined(label) || unconfined(flabel) ||
+           (!denied && aa_label_is_subset(flabel, label)))
+               goto done;
+
+       /* TODO: label cross check */
+
+       if (file->f_path.mnt && path_mediated_fs(file->f_path.dentry))
+               error = aa_path_perm(op, labels_profile(label), &file->f_path,
+                                    PATH_DELEGATE_DELETED, request, &cond);
 
-       return aa_path_perm(op, profile, &file->f_path, PATH_DELEGATE_DELETED,
-                           request, &cond);
+done:
+       rcu_read_unlock();
+
+       return error;
 }
 
 static void revalidate_tty(struct aa_label *label)
@@ -469,8 +503,7 @@ static void revalidate_tty(struct aa_label *label)
                                             struct tty_file_private, list);
                file = file_priv->file;
 
-               if (aa_file_perm(OP_INHERIT, labels_profile(label), file,
-                                MAY_READ | MAY_WRITE))
+               if (aa_file_perm(OP_INHERIT, label, file, MAY_READ | MAY_WRITE))
                        drop_tty = 1;
        }
        spin_unlock(&tty->files_lock);
@@ -484,8 +517,7 @@ static int match_file(const void *p, struct file *file, unsigned int fd)
 {
        struct aa_label *label = (struct aa_label *)p;
 
-       if (aa_file_perm(OP_INHERIT, labels_profile(label), file,
-                        aa_map_file_to_perms(file)))
+       if (aa_file_perm(OP_INHERIT, label, file, aa_map_file_to_perms(file)))
                return fd + 1;
        return 0;
 }

diff --git a/security/apparmor/include/file.h b/security/apparmor/include/file.h
index df76c208473a9c978df4e03c2821d6674a8a17f8..415512771bff12c892c1f7aa5ddc848498fdecae 100644 (file)
--- a/security/apparmor/include/file.h
+++ b/security/apparmor/include/file.h
@@ -15,6 +15,8 @@
 #ifndef __AA_FILE_H
 #define __AA_FILE_H
 
+#include <linux/spinlock.h>
+
 #include "domain.h"
 #include "match.h"
 #include "perms.h"
@@ -33,13 +35,13 @@ struct path;
 #define file_ctx(X) ((struct aa_file_ctx *)(X)->f_security)
 
 /* struct aa_file_ctx - the AppArmor context the file was opened in
+ * @lock: lock to update the ctx
+ * @label: label currently cached on the ctx
  * @perms: the permission the file was opened with
- *
- * The file_ctx could currently be directly stored in file->f_security
- * as the profile reference is now stored in the f_cred.  However the
- * ctx struct will expand in the future so we keep the struct.
  */
 struct aa_file_ctx {
+       spinlock_t lock;
+       struct aa_label __rcu *label;
        u32 allow;
 };
 
@@ -50,12 +52,16 @@ struct aa_file_ctx {
  *
  * Returns: file_ctx or NULL on failure
  */
-static inline struct aa_file_ctx *aa_alloc_file_ctx(gfp_t gfp)
+static inline struct aa_file_ctx *aa_alloc_file_ctx(struct aa_label *label,
+                                                   gfp_t gfp)
 {
        struct aa_file_ctx *ctx;
 
        ctx = kzalloc(sizeof(struct aa_file_ctx), gfp);
-
+       if (ctx) {
+               spin_lock_init(&ctx->lock);
+               rcu_assign_pointer(ctx->label, aa_get_label(label));
+       }
        return ctx;
 }
 
@@ -65,8 +71,15 @@ static inline struct aa_file_ctx *aa_alloc_file_ctx(gfp_t gfp)
  */
 static inline void aa_free_file_ctx(struct aa_file_ctx *ctx)
 {
-       if (ctx)
+       if (ctx) {
+               aa_put_label(rcu_access_pointer(ctx->label));
                kzfree(ctx);
+       }
+}
+
+static inline struct aa_label *aa_get_file_label(struct aa_file_ctx *ctx)
+{
+       return aa_get_label_rcu(&ctx->label);
 }
 
 /*
@@ -183,7 +196,7 @@ int aa_path_perm(const char *op, struct aa_profile *profile,
 int aa_path_link(struct aa_profile *profile, struct dentry *old_dentry,
                 const struct path *new_dir, struct dentry *new_dentry);
 
-int aa_file_perm(const char *op, struct aa_profile *profile, struct file *file,
+int aa_file_perm(const char *op, struct aa_label *label, struct file *file,
                 u32 request);
 
 void aa_inherit_files(const struct cred *cred, struct files_struct *files);

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index bf28b48bf6dd4a7b63e41a1c2b77848ba47f49fd..011fbb0096635ab1d0ba6c8958d8b671f9847a3b 100644 (file)
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -433,7 +433,7 @@ static int apparmor_file_alloc_security(struct file *file)
 
        /* freed by apparmor_file_free_security */
        struct aa_label *label = begin_current_label_crit_section();
-       file->f_security = aa_alloc_file_ctx(GFP_KERNEL);
+       file->f_security = aa_alloc_file_ctx(label, GFP_KERNEL);
        if (!file_ctx(file))
                error = -ENOMEM;
        end_current_label_crit_section(label);
@@ -448,33 +448,15 @@ static void apparmor_file_free_security(struct file *file)
 
 static int common_file_perm(const char *op, struct file *file, u32 mask)
 {
-       struct aa_file_ctx *fctx = file->f_security;
-       struct aa_label *label, *flabel;
+       struct aa_label *label;
        int error = 0;
 
        /* don't reaudit files closed during inheritance */
        if (file->f_path.dentry == aa_null.dentry)
                return -EACCES;
 
-       flabel = aa_cred_raw_label(file->f_cred);
-       AA_BUG(!flabel);
-
-       if (!file->f_path.mnt ||
-           !path_mediated_fs(file->f_path.dentry))
-               return 0;
-
        label = __begin_current_label_crit_section();
-
-       /* revalidate access, if task is unconfined, or the cached cred
-        * doesn't match or if the request is for more permissions than
-        * was granted.
-        *
-        * Note: the test for !unconfined(fprofile) is to handle file
-        *       delegation from unconfined tasks
-        */
-       if (!unconfined(label) && !unconfined(flabel) &&
-           ((flabel != label) || (mask & ~fctx->allow)))
-               error = aa_file_perm(op, labels_profile(label), file, mask);
+       error = aa_file_perm(op, label, file, mask);
        __end_current_label_crit_section(label);
 
        return error;