apparmor: fix x_table_lookup when stacking is not the first entry

x_table_lookup currently does stacking during label_parse() if the
target specifies a stack but its only caller ensures that it will
never be used with stacking.

Refactor to slightly simplify the code in x_to_label(), this
also fixes a long standing problem where x_to_labels check on stacking
is only on the first element to the table option list, instead of
the element that is found and used.

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

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 6308065737933f4509b05134e90a77891c3dc258..b9c29909737216922ee85ea637028aa3dde206b0 100644 (file)
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -509,6 +509,7 @@ static const char *next_name(int xtype, const char *name)
  * @name: returns: name tested to find label (NOT NULL)
  *
  * Returns: refcounted label, or NULL on failure (MAYBE NULL)
+ *          @name will always be set with the last name tried
  */
 struct aa_label *x_table_lookup(struct aa_profile *profile, u32 xindex,
                                const char **name)
@@ -518,6 +519,7 @@ struct aa_label *x_table_lookup(struct aa_profile *profile, u32 xindex,
        struct aa_label *label = NULL;
        u32 xtype = xindex & AA_X_TYPE_MASK;
        int index = xindex & AA_X_INDEX_MASK;
+       const char *next;
 
        AA_BUG(!name);
 
@@ -525,25 +527,27 @@ struct aa_label *x_table_lookup(struct aa_profile *profile, u32 xindex,
        /* TODO: move lookup parsing to unpack time so this is a straight
         *       index into the resultant label
         */
-       for (*name = rules->file->trans.table[index]; !label && *name;
-            *name = next_name(xtype, *name)) {
+       for (next = rules->file->trans.table[index]; next;
+            next = next_name(xtype, next)) {
+               const char *lookup = (*next == '&') ? next + 1 : next;
+               *name = next;
                if (xindex & AA_X_CHILD) {
-                       struct aa_profile *new_profile;
-                       /* release by caller */
-                       new_profile = aa_find_child(profile, *name);
-                       if (new_profile)
-                               label = &new_profile->label;
+                       /* TODO: switich to parse to get stack of child */
+                       struct aa_profile *new = aa_find_child(profile, lookup);
+
+                       if (new)
+                               /* release by caller */
+                               return &new->label;
                        continue;
                }
-               label = aa_label_parse(&profile->label, *name, GFP_KERNEL,
+               label = aa_label_parse(&profile->label, lookup, GFP_KERNEL,
                                       true, false);
-               if (IS_ERR(label))
-                       label = NULL;
+               if (!IS_ERR_OR_NULL(label))
+                       /* release by caller */
+                       return label;
        }
 
-       /* released by caller */
-
-       return label;
+       return NULL;
 }
 
 /**
@@ -568,9 +572,9 @@ static struct aa_label *x_to_label(struct aa_profile *profile,
        struct aa_ruleset *rules = list_first_entry(&profile->rules,
                                                    typeof(*rules), list);
        struct aa_label *new = NULL;
+       struct aa_label *stack = NULL;
        struct aa_ns *ns = profile->ns;
        u32 xtype = xindex & AA_X_TYPE_MASK;
-       const char *stack = NULL;
 
        switch (xtype) {
        case AA_X_NONE:
@@ -579,13 +583,14 @@ static struct aa_label *x_to_label(struct aa_profile *profile,
                break;
        case AA_X_TABLE:
                /* TODO: fix when perm mapping done at unload */
-               stack = rules->file->trans.table[xindex & AA_X_INDEX_MASK];
-               if (*stack != '&') {
-                       /* released by caller */
-                       new = x_table_lookup(profile, xindex, lookupname);
-                       stack = NULL;
+               /* released by caller
+                * if null for both stack and direct want to try fallback
+                */
+               new = x_table_lookup(profile, xindex, lookupname);
+               if (!new || **lookupname != '&')
                        break;
-               }
+               stack = new;
+               new = NULL;
                fallthrough;    /* to X_NAME */
        case AA_X_NAME:
                if (xindex & AA_X_CHILD)
@@ -600,6 +605,7 @@ static struct aa_label *x_to_label(struct aa_profile *profile,
                break;
        }
 
+       /* fallback transition check */
        if (!new) {
                if (xindex & AA_X_INHERIT) {
                        /* (p|c|n)ix - don't change profile but do
@@ -618,12 +624,12 @@ static struct aa_label *x_to_label(struct aa_profile *profile,
                /* base the stack on post domain transition */
                struct aa_label *base = new;
 
-               new = aa_label_parse(base, stack, GFP_KERNEL, true, false);
-               if (IS_ERR(new))
-                       new = NULL;
+               new = aa_label_merge(base, stack, GFP_KERNEL);
+               /* null on error */
                aa_put_label(base);
        }
 
+       aa_put_label(stack);
        /* released by caller */
        return new;
 }