diff --git a/security/apparmor/label.c b/security/apparmor/label.c index a165cadf8249..32efef2f617a 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -796,6 +796,44 @@ bool aa_label_remove(struct aa_label *label) return res; } +enum ls_lock_class { + AA_LS_LOCK_FIRST, + AA_LS_LOCK_SECOND, +}; + +#define write_lock_irqsave_nested(L, F, SC) write_lock_irqsave(L, F) + +static void ns_ls_double_lock(struct aa_ns *ns1, struct aa_ns *ns2, + unsigned long *flags) +{ + if (likely(ns1 == ns2)) { + write_lock_irqsave(&ns1->labels.lock, *flags); + return; + } + + /* ordered by namespace hierarchy (walked in nesting order in + * labels_update. If at the same level by address order + */ + if ((ns1->level > ns2->level) || + (ns1->level == ns2->level && ns1 > ns2)) + swap(ns1, ns2); + + write_lock_irqsave_nested(&ns1->labels.lock, *flags, AA_LS_LOCK_FIRST); + write_lock_nested(&ns2->labels.lock, AA_LS_LOCK_SECOND); +} + +static void ns_ls_double_unlock(struct aa_ns *ns1, struct aa_ns *ns2, + unsigned long flags) +{ + if (likely(ns1 == ns2)) { + write_unlock_irqrestore(&ns1->labels.lock, flags); + return; + } + /* order doesn't matter on unlock, except flags restore must be last */ + write_unlock(&ns2->labels.lock); + write_unlock_irqrestore(&ns1->labels.lock, flags); +} + /** * aa_label_replace - replace a label @old with a new version @new * @old: label to replace @@ -803,36 +841,34 @@ bool aa_label_remove(struct aa_label *label) * * Returns: true if @old was in tree and replaced * else @old was not in tree, and @new was not inserted + * + * replacement can involve two different labelsets so has to be + * handled very careful, as a double lock may be required. */ bool aa_label_replace(struct aa_label *old, struct aa_label *new) { + struct aa_ns *ons = labels_ns(old); + struct aa_ns *nns = labels_ns(new); unsigned long flags; bool res; - if (name_is_shared(old, new) && labels_ns(old) == labels_ns(new)) { - write_lock_irqsave(&labels_set(old)->lock, flags); + ns_ls_double_lock(ons, nns, &flags); + if (ons == nns && name_is_shared(old, new)) { if (old->proxy != new->proxy) __proxy_share(old, new); else __aa_proxy_redirect(old, new); res = __label_replace(old, new); - write_unlock_irqrestore(&labels_set(old)->lock, flags); } else { struct aa_label *l; - struct aa_labelset *ls = labels_set(old); - write_lock_irqsave(&ls->lock, flags); + /* will redirect old proxy to new */ res = __label_remove(old, new); - if (labels_ns(old) != labels_ns(new)) { - write_unlock_irqrestore(&ls->lock, flags); - ls = labels_set(new); - write_lock_irqsave(&ls->lock, flags); - } - l = __label_insert(ls, new, true); + l = __label_insert(&nns->labels, new, true); res = (l == new); - write_unlock_irqrestore(&ls->lock, flags); aa_put_label(l); } + ns_ls_double_unlock(ons, nns, flags); return res; }