From patchwork Tue Apr 13 07:09:36 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Johansen X-Patchwork-Id: 50041 X-Patchwork-Delegate: apw@canonical.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id 0A1D2B7CF6 for ; Tue, 13 Apr 2010 17:10:30 +1000 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.69) (envelope-from ) id 1O1aGO-00064r-Eu; Tue, 13 Apr 2010 08:10:24 +0100 Received: from adelie.canonical.com ([91.189.90.139]) by chlorine.canonical.com with esmtp (Exim 4.69) (envelope-from ) id 1O1aG4-0005nb-Ac for kernel-team@lists.ubuntu.com; Tue, 13 Apr 2010 08:10:04 +0100 Received: from hutte.canonical.com ([91.189.90.181]) by adelie.canonical.com with esmtp (Exim 4.69 #1 (Debian)) id 1O1aG4-0003MZ-64; Tue, 13 Apr 2010 08:10:04 +0100 Received: from [96.225.230.137] (helo=canonical.com) by hutte.canonical.com with esmtpsa (TLS-1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.69) (envelope-from ) id 1O1aG3-0002b2-Q7; Tue, 13 Apr 2010 08:10:04 +0100 From: john.johansen@canonical.com To: kernel-team@lists.ubuntu.com Subject: [PATCH 07/11] AppArmor: fix refcount order bug that can trigger during replacement Date: Tue, 13 Apr 2010 00:09:36 -0700 Message-Id: <1271142580-26555-8-git-send-email-john.johansen@canonical.com> X-Mailer: git-send-email 1.7.0 In-Reply-To: <1271142580-26555-1-git-send-email-john.johansen@canonical.com> References: <1271142580-26555-1-git-send-email-john.johansen@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.9 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: kernel-team-bounces@lists.ubuntu.com Errors-To: kernel-team-bounces@lists.ubuntu.com From: John Johansen OriginalAuthor: John Johansen OriginalLocation: git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparm$ commit: b96b66094d397cdf887abb77850e075c2d99b91b BugLink: http://bugs.launchpad.net/bugs/367499 AppArmor uses lazy reference counting on chains of reference to reduce refcounting overhead. When loading a replacement profile races with a task replacing the cred's current reference, it is possible that cxt->profile is the reference at the head of the replacement chain, dropping this reference can actually trigger the loss of the last profile in the chain if there are no more references to it. Signed-off-by: John Johansen Acked-by: Andy Whitcroft --- security/apparmor/context.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/security/apparmor/context.c b/security/apparmor/context.c index caaa277..14a0e41 100644 --- a/security/apparmor/context.c +++ b/security/apparmor/context.c @@ -91,8 +91,13 @@ static void replace_group(struct aa_task_cxt *cxt, struct aa_profile *profile) cxt->onexec = NULL; cxt->token = 0; } + /* be careful switching cxt->profile, when racing replacement it + * is possible that cxt->profile->replacedby is the reference keeping + * @profile valid, so make sure to get its reference before dropping + * the reference on cxt->profile */ + aa_get_profile(profile); aa_put_profile(cxt->profile); - cxt->profile = aa_get_profile(profile); + cxt->profile = profile; } /** @@ -130,8 +135,9 @@ int aa_set_current_onexec(struct aa_profile *profile) return -ENOMEM; cxt = new->security; + aa_get_profile(profile); aa_put_profile(cxt->onexec); - cxt->onexec = aa_get_profile(profile); + cxt->onexec = profile; commit_creds(new); return 0;