From e23c92aef9ec54a6efc1376fcca51b111d13863c Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 18 Mar 2016 06:05:55 -0700 Subject: [PATCH] Revert "UBUNTU: SAUCE: apparmor: Fix: refcount race between locating in labelset and get" BugLink: http://bugs.launchpad.net/bugs/1379535 This reverts commit 3ceb6a970f5eec7580ec740ed493b07c142984f2. Signed-off-by: Tim Gardner --- security/apparmor/label.c | 90 +++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 50 deletions(-) diff --git a/security/apparmor/label.c b/security/apparmor/label.c index b40c877a1196..c4e0e11b87ee 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -391,8 +391,7 @@ static struct aa_label *__aa_label_insert(struct aa_labelset *ls, static struct aa_label *__aa_label_remove_and_insert(struct aa_labelset *ls, struct aa_label *remove, - struct aa_label *insert, - bool replace) + struct aa_label *insert) { AA_BUG(!ls); AA_BUG(!remove); @@ -402,7 +401,7 @@ static struct aa_label *__aa_label_remove_and_insert(struct aa_labelset *ls, AA_BUG(insert->flags & FLAG_IN_TREE); __aa_label_remove(ls, remove); - return __aa_label_insert(ls, insert, replace); + return __aa_label_insert(ls, insert, false); } struct aa_label *aa_label_remove_and_insert(struct aa_labelset *ls, @@ -413,7 +412,7 @@ struct aa_label *aa_label_remove_and_insert(struct aa_labelset *ls, struct aa_label *l; write_lock_irqsave(&ls->lock, flags); - l = __aa_label_remove_and_insert(ls, remove, insert, false); + l = aa_get_label(__aa_label_remove_and_insert(ls, remove, insert)); write_unlock_irqrestore(&ls->lock, flags); return l; @@ -436,10 +435,12 @@ bool aa_label_replace(struct aa_labelset *ls, struct aa_label *old, bool res; write_lock_irqsave(&ls->lock, flags); - l = __aa_label_remove_and_insert(ls, old, new, true); + if (!(old->flags & FLAG_IN_TREE)) + l = __aa_label_insert(ls, new, false); + else + l = __aa_label_remove_and_insert(ls, old, new); res = (l == new); write_unlock_irqrestore(&ls->lock, flags); - aa_put_label(l); return res; } @@ -548,8 +549,7 @@ static int label_cmp(struct aa_label *a, struct aa_label *b) * Requires: @ls lock held * caller to hold a valid ref on l * - * Returns: ref counted @label if matching label is in tree - * ref counted label that is equiv to @l in tree + * Returns: unref counted @label if matching label is in tree * else NULL if @vec equiv is not in tree */ static struct aa_label *__aa_label_vec_find(struct aa_labelset *ls, @@ -572,7 +572,7 @@ static struct aa_label *__aa_label_vec_find(struct aa_labelset *ls, else if (result < 0) node = node->rb_right; else - return aa_get_label_not0(this); + return this; } return NULL; @@ -586,8 +586,8 @@ static struct aa_label *__aa_label_vec_find(struct aa_labelset *ls, * Requires: @ls lock held * caller to hold a valid ref on l * - * Returns: ref counted @l if @l is in tree OR - * ref counted label that is equiv to @l in tree + * Returns: unref counted @l if @l is in tree + * unref counted label that is equiv to @l in tree * else NULL if @l or equiv is not in tree */ static struct aa_label *__aa_label_find(struct aa_labelset *ls, @@ -620,7 +620,7 @@ struct aa_label *aa_label_vec_find(struct aa_labelset *ls, AA_BUG(n <= 0); read_lock_irqsave(&ls->lock, flags); - label = __aa_label_vec_find(ls, vec, n); + label = aa_get_label(__aa_label_vec_find(ls, vec, n)); labelstats_inc(sread); read_unlock_irqrestore(&ls->lock, flags); @@ -653,9 +653,8 @@ struct aa_label *aa_label_find(struct aa_labelset *ls, struct aa_label *l) * Requires: @ls->lock * caller to hold a valid ref on l * - * Returns: @l if successful in inserting @l - with additional refcount - * else ref counted equivalent label that is already in the set, - the else condition only happens if @replace is false + * Returns: @l if successful in inserting @l + * else ref counted equivalent label that is already in the set. */ static struct aa_label *__aa_label_insert(struct aa_labelset *ls, struct aa_label *l, bool replace) @@ -676,11 +675,10 @@ static struct aa_label *__aa_label_insert(struct aa_labelset *ls, parent = *new; if (result == 0) { labelsetstats_inc(ls, existing); - if (!replace && aa_get_label_not0(this)) + if (!replace) return this; - /* *this is either queued for destruction or being replaced */ - AA_BUG(!__aa_label_replace(ls, this, l)); - return aa_get_label(l); + __aa_label_replace(ls, this, l); + return l; } else if (result < 0) new = &((*new)->rb_left); else /* (result > 0) */ @@ -694,7 +692,7 @@ static struct aa_label *__aa_label_insert(struct aa_labelset *ls, labelsetstats_inc(ls, insert); labelsetstats_inc(ls, intree); - return aa_get_label(l); + return l; } /** @@ -718,7 +716,7 @@ struct aa_label *aa_label_insert(struct aa_labelset *ls, struct aa_label *l) /* check if label exists before taking lock */ if (!label_invalid(l)) { read_lock_irqsave(&ls->lock, flags); - label = __aa_label_find(ls, l); + label = aa_get_label(__aa_label_find(ls, l)); read_unlock_irqrestore(&ls->lock, flags); labelstats_inc(fread); if (label) @@ -726,7 +724,7 @@ struct aa_label *aa_label_insert(struct aa_labelset *ls, struct aa_label *l) } write_lock_irqsave(&ls->lock, flags); - label = __aa_label_insert(ls, l, false); + label = aa_get_label(__aa_label_insert(ls, l, false)); write_unlock_irqrestore(&ls->lock, flags); return label; @@ -957,7 +955,9 @@ static struct aa_label *__label_merge_insert(struct aa_labelset *ls, } if (label_profiles_unconfined(l)) l->flags |= FLAG_UNCONFINED; - return __aa_label_insert(ls, l, true); + __aa_label_insert(ls, l, true); + + return aa_get_label(l); } /** @@ -985,7 +985,7 @@ static struct aa_labelset *labelset_of_merge(struct aa_label *a, struct aa_label * * Requires: read_lock held * - * Returns: ref counted label that is equiv to merge of @a and @b + * Returns: unref counted label that is equiv to merge of @a and @b * else NULL if merge of @a and @b is not in set */ static struct aa_label *__aa_label_find_merge(struct aa_labelset *ls, @@ -1012,7 +1012,7 @@ static struct aa_label *__aa_label_find_merge(struct aa_labelset *ls, else if (result > 0) node = node->rb_right; else - return aa_get_label_not0(this); + return this; } return NULL; @@ -1044,7 +1044,7 @@ struct aa_label *aa_label_find_merge(struct aa_label *a, struct aa_label *b) a = ar = aa_get_newest_label(a); if (label_invalid(b)) b = br = aa_get_newest_label(b); - label = __aa_label_find_merge(ls, a, b); + label = aa_get_label(__aa_label_find_merge(ls, a, b)); read_unlock_irqrestore(&ls->lock, flags); aa_put_label(ar); aa_put_label(br); @@ -1145,14 +1145,13 @@ struct aa_label *aa_label_vec_merge(struct aa_profile **vec, int len, for (i = 0; i < len; i++) { new->ent[i] = aa_get_profile(vec[i]); label = __aa_label_insert(ls, new, false); + if (label != new) { + aa_get_label(label); + /* not fully constructed don't put */ + aa_label_free(new); + } } write_unlock_irqrestore(&ls->lock, flags); - if (label != new) - /* not fully constructed don't put */ - aa_label_free(new); - else - /* extra ref */ - aa_put_label(new); return label; } @@ -1702,18 +1701,6 @@ void aa_labelset_init(struct aa_labelset *ls) labelstats_init(&ls); } -static bool vec_invalid(struct aa_profile **vec, int n) -{ - int i; - - for (i = 0; i < n; i++) { - if (PROFILE_INVALID(vec[i])) - return true; - } - - return false; -} - static struct aa_label *labelset_next_invalid(struct aa_labelset *ls) { struct aa_label *label; @@ -1729,14 +1716,18 @@ static struct aa_label *labelset_next_invalid(struct aa_labelset *ls) struct label_it i; label = rb_entry(node, struct aa_label, node); - if ((label_invalid(label) || vec_invalid(label->ent, label->size)) && - aa_get_label_not0(label)) + if (label_invalid(label)) goto out; + label_for_each(i, label, p) { + if (PROFILE_INVALID(p)) + goto out; + } } label = NULL; out: + aa_get_label(label); read_unlock_irqrestore(&ls->lock, flags); return label; @@ -1810,12 +1801,11 @@ static struct aa_label *__label_update(struct aa_label *label) goto insert; } insert: - tmp = __aa_label_remove_and_insert(labels_set(label), label, l, true); + tmp = __aa_label_remove_and_insert(labels_set(label), label, l); if (tmp != l) { + aa_get_label(tmp); aa_label_free(l); - l = tmp; - } else - aa_put_label(l); /* extr ref */ + } write_unlock_irqrestore(&ls->lock, flags); -- 2.39.5