From patchwork Wed Dec 2 14:59:33 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Whitcroft X-Patchwork-Id: 551396 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 DE6BC140313; Thu, 3 Dec 2015 01:59:59 +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 1a48sw-0000wt-0C; Wed, 02 Dec 2015 14:59:58 +0000 Received: from mail-wm0-f46.google.com ([74.125.82.46]) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1a48sj-0000rh-Lj for kernel-team@lists.ubuntu.com; Wed, 02 Dec 2015 14:59:45 +0000 Received: by wmvv187 with SMTP id v187so259055145wmv.1 for ; Wed, 02 Dec 2015 06:59:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=406vh6oJjK2M897pX8Ds/OlBsk+6+Els2tb0VL5sp+M=; b=Tukqre0wHeaYU9q+C5mbTOt3IStW4A/tmdFuhYMZCE3VJW9gMIAbUR3t8f09/VeCys UhrcS+W+h3K5SbXMgqWpnolrXUxDweOvZuinOzLJ9e0rabR+2OQsTAJdGiiFWxoT5i+x N+zbkbfGqXBP/Yta2sbdififKGuF4l9PQmq6XTRMy52hKsi+DAavzvlqfLepafYKmV6S AMBC0d7E8VyJjH2yQdhdjW/DLJyhHw7yGR+B+j3ccLkMTGNBO3GEi9cgd1yv1iLi0LVp f7rbI7AoIgddoOd0rUbXQyynq6tKtuvXJANtzmY4hLlsh77NFzmYc3eM1nD70Gf6+NWp wSEw== X-Gm-Message-State: ALoCoQljQQZ8g4h7IQKbwNli5ltQLHuVKN/Y9ItiJW06wrAdrDEfP4gWFOU4wMQKkO9BTTQS1mcx X-Received: by 10.194.134.135 with SMTP id pk7mr5359878wjb.111.1449068385518; Wed, 02 Dec 2015 06:59:45 -0800 (PST) Received: from localhost ([2001:470:6973:2:55ab:1437:1293:1d55]) by smtp.gmail.com with ESMTPSA id k133sm3574900wmg.18.2015.12.02.06.59.44 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 02 Dec 2015 06:59:44 -0800 (PST) From: Andy Whitcroft To: kernel-team@lists.ubuntu.com Subject: [wily/master-next 3/7] KEYS: Fix race between key destruction and finding a keyring by name Date: Wed, 2 Dec 2015 14:59:33 +0000 Message-Id: <1449068377-21867-4-git-send-email-apw@canonical.com> X-Mailer: git-send-email 2.6.2 In-Reply-To: <1449068377-21867-1-git-send-email-apw@canonical.com> References: <1449068377-21867-1-git-send-email-apw@canonical.com> Cc: Andy Whitcroft 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 From: David Howells There appears to be a race between: (1) key_gc_unused_keys() which frees key->security and then calls keyring_destroy() to unlink the name from the name list (2) find_keyring_by_name() which calls key_permission(), thus accessing key->security, on a key before checking to see whether the key usage is 0 (ie. the key is dead and might be cleaned up). Fix this by calling ->destroy() before cleaning up the core key data - including key->security. Reported-by: Petr Matousek Signed-off-by: David Howells (cherry picked from commit 94c4554ba07adbdde396748ee7ae01e86cf2d8d7) CVE-2015-7872 BugLink: http://bugs.launchpad.net/bugs/1508856 Signed-off-by: Andy Whitcroft --- security/keys/gc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/security/keys/gc.c b/security/keys/gc.c index c795237..39eac1f 100644 --- a/security/keys/gc.c +++ b/security/keys/gc.c @@ -134,6 +134,10 @@ static noinline void key_gc_unused_keys(struct list_head *keys) kdebug("- %u", key->serial); key_check(key); + /* Throw away the key data */ + if (key->type->destroy) + key->type->destroy(key); + security_key_free(key); /* deal with the user's key tracking and quota */ @@ -148,10 +152,6 @@ static noinline void key_gc_unused_keys(struct list_head *keys) if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) atomic_dec(&key->user->nikeys); - /* now throw away the key memory */ - if (key->type->destroy) - key->type->destroy(key); - key_user_put(key->user); kfree(key->description);