From patchwork Fri Mar 31 12:57:37 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Johansen X-Patchwork-Id: 745625 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 3vvhPn3ByFz9s03; Fri, 31 Mar 2017 23:58:21 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1ctw8A-0000kj-67; Fri, 31 Mar 2017 12:58:18 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.76) (envelope-from ) id 1ctw84-0000iY-2J for kernel-team@lists.ubuntu.com; Fri, 31 Mar 2017 12:58:12 +0000 Received: from static-50-53-32-2.bvtn.or.frontiernet.net ([50.53.32.2] helo=canonical.com) by youngberry.canonical.com with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.76) (envelope-from ) id 1ctw83-0006Ab-GA for kernel-team@lists.ubuntu.com; Fri, 31 Mar 2017 12:58:11 +0000 From: John Johansen To: kernel-team@lists.ubuntu.com Subject: [PATCH 04/11] UBUNTU: SAUCE: apparmor: fix label leak when new label is unused Date: Fri, 31 Mar 2017 05:57:37 -0700 Message-Id: <20170331125744.16986-5-john.johansen@canonical.com> X-Mailer: git-send-email 2.9.3 In-Reply-To: <20170331125744.16986-1-john.johansen@canonical.com> References: <20170331125744.16986-1-john.johansen@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com When a new label is created, it is created with a proxy in a circular ref count that is broken by replacement. However if the label is not used it will never be replaced and the circular ref count will never be broken resulting in a leak. BugLink: http://bugs.launchpad.net/bugs/1660834 Signed-off-by: John Johansen Acked-by: Stefan Bader Acked-by: Tim Gardner Acked-by: Brad Figg Signed-off-by: Thadeu Lima de Souza Cascardo --- security/apparmor/label.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/security/apparmor/label.c b/security/apparmor/label.c index 05ac6d6..69a600b4 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -308,6 +308,8 @@ out: static void label_destroy(struct aa_label *label) { + struct aa_label *tmp; + AA_BUG(!label); if (label_is_stale(label)) @@ -329,6 +331,11 @@ static void label_destroy(struct aa_label *label) rcu_assign_pointer(label->proxy->label, NULL); aa_free_sid(label->sid); + + tmp = rcu_dereference_protected(label->proxy->label, true); + if (tmp == label) + rcu_assign_pointer(label->proxy->label, NULL); + aa_put_proxy(label->proxy); label->proxy = (struct aa_proxy *) PROXY_POISON + 1; } @@ -380,6 +387,15 @@ void aa_label_kref(struct kref *kref) call_rcu(&label->rcu, label_free_rcu); } +static void label_free_or_put_new(struct aa_label *label, struct aa_label *new) +{ + if (label != new) + /* need to free directly to break circular ref with proxy */ + aa_label_free(new); + else + aa_put_label(new); +} + bool aa_label_init(struct aa_label *label, int size) { AA_BUG(!label); @@ -842,7 +858,7 @@ static struct aa_label *vec_create_and_insert_label(struct aa_profile **vec, write_lock_irqsave(&ls->lock, flags); label = __label_insert(ls, new, false); write_unlock_irqrestore(&ls->lock, flags); - aa_put_label(new); + label_free_or_put_new(label, new); return label; } @@ -1241,7 +1257,7 @@ struct aa_label *aa_label_merge(struct aa_label *a, struct aa_label *b, goto out; label = label_merge_insert(new, a, b); - aa_put_label(new); + label_free_or_put_new(label, new); out: aa_put_label(a); aa_put_label(b); @@ -2042,8 +2058,7 @@ remove: /* ensure label is removed, and redirected correctly */ __label_remove(label, tmp); write_unlock_irqrestore(&ls->lock, flags); - - aa_put_label(new); + label_free_or_put_new(tmp, new); return tmp; }