Merge tag 'apparmor-pr-2017-11-21' of git://git.kernel.org/pub/scm/linux/kernel/git...
authorLinus Torvalds <torvalds@linux-foundation.org>
Fri, 24 Nov 2017 06:48:26 +0000 (20:48 -1000)
committerLinus Torvalds <torvalds@linux-foundation.org>
Fri, 24 Nov 2017 06:48:26 +0000 (20:48 -1000)
Pull apparmor updates from John Johansen:
 "No features this time, just minor cleanups and bug fixes.

  Cleanups:
   - fix spelling mistake: "resoure" -> "resource"
   - remove unused redundant variable stop
   - Fix bool initialization/comparison

  Bug Fixes:
   - initialized returned struct aa_perms
   - fix leak of null profile name if profile allocation fails
   - ensure that undecidable profile attachments fail
   - fix profile attachment for special unconfined profiles
   - fix locking when creating a new complain profile.
   - fix possible recursive lock warning in __aa_create_ns"

* tag 'apparmor-pr-2017-11-21' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor:
  apparmor: fix possible recursive lock warning in __aa_create_ns
  apparmor: fix locking when creating a new complain profile.
  apparmor: fix profile attachment for special unconfined profiles
  apparmor: ensure that undecidable profile attachments fail
  apparmor: fix leak of null profile name if profile allocation fails
  apparmor: remove unused redundant variable stop
  apparmor: Fix bool initialization/comparison
  apparmor: initialized returned struct aa_perms
  apparmor: fix spelling mistake: "resoure" -> "resource"

security/apparmor/apparmorfs.c
security/apparmor/domain.c
security/apparmor/file.c
security/apparmor/label.c
security/apparmor/lib.c
security/apparmor/lsm.c
security/apparmor/mount.c
security/apparmor/policy.c
security/apparmor/policy_ns.c
security/apparmor/policy_unpack.c
security/apparmor/resource.c

index caaf51dda64812067e07d1c9079c7750c4296e11..8542e9a55e1b8c718eca5f267a1bb71195599774 100644 (file)
@@ -533,7 +533,7 @@ static ssize_t ns_revision_read(struct file *file, char __user *buf,
        long last_read;
        int avail;
 
-       mutex_lock(&rev->ns->lock);
+       mutex_lock_nested(&rev->ns->lock, rev->ns->level);
        last_read = rev->last_read;
        if (last_read == rev->ns->revision) {
                mutex_unlock(&rev->ns->lock);
@@ -543,7 +543,7 @@ static ssize_t ns_revision_read(struct file *file, char __user *buf,
                                             last_read !=
                                             READ_ONCE(rev->ns->revision)))
                        return -ERESTARTSYS;
-               mutex_lock(&rev->ns->lock);
+               mutex_lock_nested(&rev->ns->lock, rev->ns->level);
        }
 
        avail = sprintf(buffer, "%ld\n", rev->ns->revision);
@@ -577,7 +577,7 @@ static unsigned int ns_revision_poll(struct file *file, poll_table *pt)
        unsigned int mask = 0;
 
        if (rev) {
-               mutex_lock(&rev->ns->lock);
+               mutex_lock_nested(&rev->ns->lock, rev->ns->level);
                poll_wait(file, &rev->ns->wait, pt);
                if (rev->last_read < rev->ns->revision)
                        mask |= POLLIN | POLLRDNORM;
@@ -1643,7 +1643,7 @@ static int ns_mkdir_op(struct inode *dir, struct dentry *dentry, umode_t mode)
         */
        inode_unlock(dir);
        error = simple_pin_fs(&aafs_ops, &aafs_mnt, &aafs_count);
-       mutex_lock(&parent->lock);
+       mutex_lock_nested(&parent->lock, parent->level);
        inode_lock_nested(dir, I_MUTEX_PARENT);
        if (error)
                goto out;
@@ -1692,7 +1692,7 @@ static int ns_rmdir_op(struct inode *dir, struct dentry *dentry)
        inode_unlock(dir);
        inode_unlock(dentry->d_inode);
 
-       mutex_lock(&parent->lock);
+       mutex_lock_nested(&parent->lock, parent->level);
        ns = aa_get_ns(__aa_findn_ns(&parent->sub_ns, dentry->d_name.name,
                                     dentry->d_name.len));
        if (!ns) {
@@ -1747,7 +1747,7 @@ void __aafs_ns_rmdir(struct aa_ns *ns)
                __aafs_profile_rmdir(child);
 
        list_for_each_entry(sub, &ns->sub_ns, base.list) {
-               mutex_lock(&sub->lock);
+               mutex_lock_nested(&sub->lock, sub->level);
                __aafs_ns_rmdir(sub);
                mutex_unlock(&sub->lock);
        }
@@ -1877,7 +1877,7 @@ int __aafs_ns_mkdir(struct aa_ns *ns, struct dentry *parent, const char *name,
 
        /* subnamespaces */
        list_for_each_entry(sub, &ns->sub_ns, base.list) {
-               mutex_lock(&sub->lock);
+               mutex_lock_nested(&sub->lock, sub->level);
                error = __aafs_ns_mkdir(sub, ns_subns_dir(ns), NULL, NULL);
                mutex_unlock(&sub->lock);
                if (error)
@@ -1921,7 +1921,7 @@ static struct aa_ns *__next_ns(struct aa_ns *root, struct aa_ns *ns)
        /* is next namespace a child */
        if (!list_empty(&ns->sub_ns)) {
                next = list_first_entry(&ns->sub_ns, typeof(*ns), base.list);
-               mutex_lock(&next->lock);
+               mutex_lock_nested(&next->lock, next->level);
                return next;
        }
 
@@ -1931,7 +1931,7 @@ static struct aa_ns *__next_ns(struct aa_ns *root, struct aa_ns *ns)
                mutex_unlock(&ns->lock);
                next = list_next_entry(ns, base.list);
                if (!list_entry_is_head(next, &parent->sub_ns, base.list)) {
-                       mutex_lock(&next->lock);
+                       mutex_lock_nested(&next->lock, next->level);
                        return next;
                }
                ns = parent;
@@ -2039,7 +2039,7 @@ static void *p_start(struct seq_file *f, loff_t *pos)
        f->private = root;
 
        /* find the first profile */
-       mutex_lock(&root->lock);
+       mutex_lock_nested(&root->lock, root->level);
        profile = __first_profile(root, root);
 
        /* skip to position */
@@ -2491,7 +2491,7 @@ static int __init aa_create_aafs(void)
        ns_subrevision(root_ns) = dent;
 
        /* policy tree referenced by magic policy symlink */
-       mutex_lock(&root_ns->lock);
+       mutex_lock_nested(&root_ns->lock, root_ns->level);
        error = __aafs_ns_mkdir(root_ns, aafs_mnt->mnt_root, ".policy",
                                aafs_mnt->mnt_root);
        mutex_unlock(&root_ns->lock);
index dd754b7850a82b4d129c11de0c55603de19268ba..04ba9d0718ea590b7c5033cfc4b952c701acb54d 100644 (file)
@@ -305,6 +305,7 @@ static int change_profile_perms(struct aa_profile *profile,
  * __attach_match_ - find an attachment match
  * @name - to match against  (NOT NULL)
  * @head - profile list to walk  (NOT NULL)
+ * @info - info message if there was an error (NOT NULL)
  *
  * Do a linear search on the profiles in the list.  There is a matching
  * preference where an exact match is preferred over a name which uses
@@ -316,28 +317,46 @@ static int change_profile_perms(struct aa_profile *profile,
  * Returns: profile or NULL if no match found
  */
 static struct aa_profile *__attach_match(const char *name,
-                                        struct list_head *head)
+                                        struct list_head *head,
+                                        const char **info)
 {
        int len = 0;
+       bool conflict = false;
        struct aa_profile *profile, *candidate = NULL;
 
        list_for_each_entry_rcu(profile, head, base.list) {
-               if (profile->label.flags & FLAG_NULL)
+               if (profile->label.flags & FLAG_NULL &&
+                   &profile->label == ns_unconfined(profile->ns))
                        continue;
-               if (profile->xmatch && profile->xmatch_len > len) {
-                       unsigned int state = aa_dfa_match(profile->xmatch,
-                                                         DFA_START, name);
-                       u32 perm = dfa_user_allow(profile->xmatch, state);
-                       /* any accepting state means a valid match. */
-                       if (perm & MAY_EXEC) {
-                               candidate = profile;
-                               len = profile->xmatch_len;
+
+               if (profile->xmatch) {
+                       if (profile->xmatch_len == len) {
+                               conflict = true;
+                               continue;
+                       } else if (profile->xmatch_len > len) {
+                               unsigned int state;
+                               u32 perm;
+
+                               state = aa_dfa_match(profile->xmatch,
+                                                    DFA_START, name);
+                               perm = dfa_user_allow(profile->xmatch, state);
+                               /* any accepting state means a valid match. */
+                               if (perm & MAY_EXEC) {
+                                       candidate = profile;
+                                       len = profile->xmatch_len;
+                                       conflict = false;
+                               }
                        }
                } else if (!strcmp(profile->base.name, name))
                        /* exact non-re match, no more searching required */
                        return profile;
        }
 
+       if (conflict) {
+               *info = "conflicting profile attachments";
+               return NULL;
+       }
+
        return candidate;
 }
 
@@ -346,16 +365,17 @@ static struct aa_profile *__attach_match(const char *name,
  * @ns: the current namespace  (NOT NULL)
  * @list: list to search  (NOT NULL)
  * @name: the executable name to match against  (NOT NULL)
+ * @info: info message if there was an error
  *
  * Returns: label or NULL if no match found
  */
 static struct aa_label *find_attach(struct aa_ns *ns, struct list_head *list,
-                                   const char *name)
+                                   const char *name, const char **info)
 {
        struct aa_profile *profile;
 
        rcu_read_lock();
-       profile = aa_get_profile(__attach_match(name, list));
+       profile = aa_get_profile(__attach_match(name, list, info));
        rcu_read_unlock();
 
        return profile ? &profile->label : NULL;
@@ -448,11 +468,11 @@ static struct aa_label *x_to_label(struct aa_profile *profile,
                if (xindex & AA_X_CHILD)
                        /* released by caller */
                        new = find_attach(ns, &profile->base.profiles,
-                                               name);
+                                         name, info);
                else
                        /* released by caller */
                        new = find_attach(ns, &ns->base.profiles,
-                                               name);
+                                         name, info);
                *lookupname = name;
                break;
        }
@@ -516,7 +536,7 @@ static struct aa_label *profile_transition(struct aa_profile *profile,
 
        if (profile_unconfined(profile)) {
                new = find_attach(profile->ns, &profile->ns->base.profiles,
-                                 name);
+                                 name, &info);
                if (new) {
                        AA_DEBUG("unconfined attached to new label");
                        return new;
@@ -541,9 +561,21 @@ static struct aa_label *profile_transition(struct aa_profile *profile,
                }
        } else if (COMPLAIN_MODE(profile)) {
                /* no exec permission - learning mode */
-               struct aa_profile *new_profile = aa_new_null_profile(profile,
-                                                             false, name,
-                                                             GFP_ATOMIC);
+               struct aa_profile *new_profile = NULL;
+               char *n = kstrdup(name, GFP_ATOMIC);
+
+               if (n) {
+                       /* name is ptr into buffer */
+                       long pos = name - buffer;
+                       /* break per cpu buffer hold */
+                       put_buffers(buffer);
+                       new_profile = aa_new_null_profile(profile, false, n,
+                                                         GFP_KERNEL);
+                       get_buffers(buffer);
+                       name = buffer + pos;
+                       strcpy((char *)name, n);
+                       kfree(n);
+               }
                if (!new_profile) {
                        error = -ENOMEM;
                        info = "could not create null profile";
index 3382518b87fa507200679cb9ef660329a292debc..e79bf44396a36f60dde2e17fc68f2d0ac7d914b9 100644 (file)
@@ -226,18 +226,12 @@ static u32 map_old_perms(u32 old)
 struct aa_perms aa_compute_fperms(struct aa_dfa *dfa, unsigned int state,
                                  struct path_cond *cond)
 {
-       struct aa_perms perms;
-
        /* FIXME: change over to new dfa format
         * currently file perms are encoded in the dfa, new format
         * splits the permissions from the dfa.  This mapping can be
         * done at profile load
         */
-       perms.deny = 0;
-       perms.kill = perms.stop = 0;
-       perms.complain = perms.cond = 0;
-       perms.hide = 0;
-       perms.prompt = 0;
+       struct aa_perms perms = { };
 
        if (uid_eq(current_fsuid(), cond->uid)) {
                perms.allow = map_old_perms(dfa_user_allow(dfa, state));
index ad28e03a6f30341ab013e23cc2641f144ab44ea1..324fe5c60f8781c952138d0a764a5c2223f6353b 100644 (file)
@@ -2115,7 +2115,7 @@ void __aa_labelset_update_subtree(struct aa_ns *ns)
        __labelset_update(ns);
 
        list_for_each_entry(child, &ns->sub_ns, base.list) {
-               mutex_lock(&child->lock);
+               mutex_lock_nested(&child->lock, child->level);
                __aa_labelset_update_subtree(child);
                mutex_unlock(&child->lock);
        }
index 08ca26bcca7703c7f74e1531879eea4dd3bf2ac9..4d5e98e49d5e06a9066f618adabe2767da45ff3c 100644 (file)
@@ -317,14 +317,11 @@ static u32 map_other(u32 x)
 void aa_compute_perms(struct aa_dfa *dfa, unsigned int state,
                      struct aa_perms *perms)
 {
-       perms->deny = 0;
-       perms->kill = perms->stop = 0;
-       perms->complain = perms->cond = 0;
-       perms->hide = 0;
-       perms->prompt = 0;
-       perms->allow = dfa_user_allow(dfa, state);
-       perms->audit = dfa_user_audit(dfa, state);
-       perms->quiet = dfa_user_quiet(dfa, state);
+       *perms = (struct aa_perms) {
+               .allow = dfa_user_allow(dfa, state),
+               .audit = dfa_user_audit(dfa, state),
+               .quiet = dfa_user_quiet(dfa, state),
+       };
 
        /* for v5 perm mapping in the policydb, the other set is used
         * to extend the general perm set
@@ -426,7 +423,6 @@ int aa_check_perms(struct aa_profile *profile, struct aa_perms *perms,
                   void (*cb)(struct audit_buffer *, void *))
 {
        int type, error;
-       bool stop = false;
        u32 denied = request & (~perms->allow | perms->deny);
 
        if (likely(!denied)) {
@@ -447,8 +443,6 @@ int aa_check_perms(struct aa_profile *profile, struct aa_perms *perms,
                else
                        type = AUDIT_APPARMOR_DENIED;
 
-               if (denied & perms->stop)
-                       stop = true;
                if (denied == (denied & perms->hide))
                        error = -ENOENT;
 
index 17893fde44873ade4c28a62b7a3729e73d7c161d..9a65eeaf7dfa22ab3b76d05b1445c56031195cd3 100644 (file)
@@ -846,7 +846,7 @@ module_param_call(audit, param_set_audit, param_get_audit,
 /* Determines if audit header is included in audited messages.  This
  * provides more context if the audit daemon is not running
  */
-bool aa_g_audit_header = 1;
+bool aa_g_audit_header = true;
 module_param_named(audit_header, aa_g_audit_header, aabool,
                   S_IRUSR | S_IWUSR);
 
@@ -871,7 +871,7 @@ module_param_named(path_max, aa_g_path_max, aauint, S_IRUSR);
  * DEPRECATED: read only as strict checking of load is always done now
  * that none root users (user namespaces) can load policy.
  */
-bool aa_g_paranoid_load = 1;
+bool aa_g_paranoid_load = true;
 module_param_named(paranoid_load, aa_g_paranoid_load, aabool, S_IRUGO);
 
 /* Boot time disable flag */
@@ -1119,7 +1119,7 @@ static int __init apparmor_init(void)
 
        if (!apparmor_enabled || !security_module_enable("apparmor")) {
                aa_info_message("AppArmor disabled by boot time parameter");
-               apparmor_enabled = 0;
+               apparmor_enabled = false;
                return 0;
        }
 
@@ -1175,7 +1175,7 @@ alloc_out:
        aa_destroy_aafs();
        aa_teardown_dfa_engine();
 
-       apparmor_enabled = 0;
+       apparmor_enabled = false;
        return error;
 }
 
index 82a64b58041d2adc62debcf572b77b7a6607678d..ed9b4d0f9f7e212b161c1a312c52b56f5e0b0b49 100644 (file)
@@ -216,13 +216,12 @@ static unsigned int match_mnt_flags(struct aa_dfa *dfa, unsigned int state,
 static struct aa_perms compute_mnt_perms(struct aa_dfa *dfa,
                                           unsigned int state)
 {
-       struct aa_perms perms;
-
-       perms.kill = 0;
-       perms.allow = dfa_user_allow(dfa, state);
-       perms.audit = dfa_user_audit(dfa, state);
-       perms.quiet = dfa_user_quiet(dfa, state);
-       perms.xindex = dfa_user_xindex(dfa, state);
+       struct aa_perms perms = {
+               .allow = dfa_user_allow(dfa, state),
+               .audit = dfa_user_audit(dfa, state),
+               .quiet = dfa_user_quiet(dfa, state),
+               .xindex = dfa_user_xindex(dfa, state),
+       };
 
        return perms;
 }
index 4243b0c3f0e4acc6d66c70ea878f32d548bebdd4..b0b58848c2487e69cca16f9bfd3ee21d466a12af 100644 (file)
@@ -502,7 +502,7 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, bool hat,
 {
        struct aa_profile *p, *profile;
        const char *bname;
-       char *name;
+       char *name = NULL;
 
        AA_BUG(!parent);
 
@@ -545,7 +545,7 @@ name:
        profile->file.dfa = aa_get_dfa(nulldfa);
        profile->policy.dfa = aa_get_dfa(nulldfa);
 
-       mutex_lock(&profile->ns->lock);
+       mutex_lock_nested(&profile->ns->lock, profile->ns->level);
        p = __find_child(&parent->base.profiles, bname);
        if (p) {
                aa_free_profile(profile);
@@ -562,6 +562,7 @@ out:
        return profile;
 
 fail:
+       kfree(name);
        aa_free_profile(profile);
        return NULL;
 }
@@ -905,7 +906,7 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label,
        } else
                ns = aa_get_ns(policy_ns ? policy_ns : labels_ns(label));
 
-       mutex_lock(&ns->lock);
+       mutex_lock_nested(&ns->lock, ns->level);
        /* check for duplicate rawdata blobs: space and file dedup */
        list_for_each_entry(rawdata_ent, &ns->rawdata_list, list) {
                if (aa_rawdata_eq(rawdata_ent, udata)) {
@@ -1116,13 +1117,13 @@ ssize_t aa_remove_profiles(struct aa_ns *policy_ns, struct aa_label *subj,
 
        if (!name) {
                /* remove namespace - can only happen if fqname[0] == ':' */
-               mutex_lock(&ns->parent->lock);
+               mutex_lock_nested(&ns->parent->lock, ns->level);
                __aa_remove_ns(ns);
                __aa_bump_ns_revision(ns);
                mutex_unlock(&ns->parent->lock);
        } else {
                /* remove profile */
-               mutex_lock(&ns->lock);
+               mutex_lock_nested(&ns->lock, ns->level);
                profile = aa_get_profile(__lookup_profile(&ns->base, name));
                if (!profile) {
                        error = -ENOENT;
index 62a3589c62ab624156c0c01eaa8cadd72276866a..b1e629cba70b76f7b586ea07bfa0b77725c73e1d 100644 (file)
@@ -256,7 +256,8 @@ static struct aa_ns *__aa_create_ns(struct aa_ns *parent, const char *name,
        ns = alloc_ns(parent->base.hname, name);
        if (!ns)
                return NULL;
-       mutex_lock(&ns->lock);
+       ns->level = parent->level + 1;
+       mutex_lock_nested(&ns->lock, ns->level);
        error = __aafs_ns_mkdir(ns, ns_subns_dir(parent), name, dir);
        if (error) {
                AA_ERROR("Failed to create interface for ns %s\n",
@@ -266,7 +267,6 @@ static struct aa_ns *__aa_create_ns(struct aa_ns *parent, const char *name,
                return ERR_PTR(error);
        }
        ns->parent = aa_get_ns(parent);
-       ns->level = parent->level + 1;
        list_add_rcu(&ns->base.list, &parent->sub_ns);
        /* add list ref */
        aa_get_ns(ns);
@@ -313,7 +313,7 @@ struct aa_ns *aa_prepare_ns(struct aa_ns *parent, const char *name)
 {
        struct aa_ns *ns;
 
-       mutex_lock(&parent->lock);
+       mutex_lock_nested(&parent->lock, parent->level);
        /* try and find the specified ns and if it doesn't exist create it */
        /* released by caller */
        ns = aa_get_ns(__aa_find_ns(&parent->sub_ns, name));
@@ -336,7 +336,7 @@ static void destroy_ns(struct aa_ns *ns)
        if (!ns)
                return;
 
-       mutex_lock(&ns->lock);
+       mutex_lock_nested(&ns->lock, ns->level);
        /* release all profiles in this namespace */
        __aa_profile_list_release(&ns->base.profiles);
 
index 4ede87c30f8b890a63e1aaae0a75a9ed6ba1045c..59a1a25b7d43f209b594d61c7fd38fb4e0e50f37 100644 (file)
@@ -157,7 +157,7 @@ static void do_loaddata_free(struct work_struct *work)
        struct aa_ns *ns = aa_get_ns(d->ns);
 
        if (ns) {
-               mutex_lock(&ns->lock);
+               mutex_lock_nested(&ns->lock, ns->level);
                __aa_fs_remove_rawdata(d);
                mutex_unlock(&ns->lock);
                aa_put_ns(ns);
index d8bc842594edd884b12412edf540f3fae5feb810..cf4d234febe94c9e96a8c0dc7df4d0132f856621 100644 (file)
@@ -47,7 +47,7 @@ static void audit_cb(struct audit_buffer *ab, void *va)
 /**
  * audit_resource - audit setting resource limit
  * @profile: profile being enforced  (NOT NULL)
- * @resoure: rlimit being auditing
+ * @resource: rlimit being auditing
  * @value: value being set
  * @error: error value
  *
@@ -128,7 +128,7 @@ int aa_task_setrlimit(struct aa_label *label, struct task_struct *task,
                error = fn_for_each(label, profile,
                                audit_resource(profile, resource,
                                               new_rlim->rlim_max, peer,
-                                              "cap_sys_resoure", -EACCES));
+                                              "cap_sys_resource", -EACCES));
        else
                error = fn_for_each_confined(label, profile,
                                profile_setrlimit(profile, resource, new_rlim));