Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751961Ab3CREoR (ORCPT ); Mon, 18 Mar 2013 00:44:17 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:33542 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751857Ab3CREae (ORCPT ); Mon, 18 Mar 2013 00:30:34 -0400 Message-Id: <20130318042149.752881564@decadent.org.uk> User-Agent: quilt/0.60-1 Date: Mon, 18 Mar 2013 04:22:41 +0000 From: Ben Hutchings To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: akpm@linux-foundation.org, David Howells , Mateusz Guzik , James Morris Subject: [ 57/82] keys: fix race with concurrent install_user_keyrings() In-Reply-To: <20130318042144.234468645@decadent.org.uk> X-SA-Exim-Connect-IP: 2001:470:1f08:1539:d98f:da4e:f620:7bea X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2762 Lines: 77 3.2-stable review patch. If anyone has any objections, please let me know. ------------------ From: David Howells commit 0da9dfdd2cd9889201bc6f6f43580c99165cd087 upstream. This fixes CVE-2013-1792. There is a race in install_user_keyrings() that can cause a NULL pointer dereference when called concurrently for the same user if the uid and uid-session keyrings are not yet created. It might be possible for an unprivileged user to trigger this by calling keyctl() from userspace in parallel immediately after logging in. Assume that we have two threads both executing lookup_user_key(), both looking for KEY_SPEC_USER_SESSION_KEYRING. THREAD A THREAD B =============================== =============================== ==>call install_user_keyrings(); if (!cred->user->session_keyring) ==>call install_user_keyrings() ... user->uid_keyring = uid_keyring; if (user->uid_keyring) return 0; <== key = cred->user->session_keyring [== NULL] user->session_keyring = session_keyring; atomic_inc(&key->usage); [oops] At the point thread A dereferences cred->user->session_keyring, thread B hasn't updated user->session_keyring yet, but thread A assumes it is populated because install_user_keyrings() returned ok. The race window is really small but can be exploited if, for example, thread B is interrupted or preempted after initializing uid_keyring, but before doing setting session_keyring. This couldn't be reproduced on a stock kernel. However, after placing systemtap probe on 'user->session_keyring = session_keyring;' that introduced some delay, the kernel could be crashed reliably. Fix this by checking both pointers before deciding whether to return. Alternatively, the test could be done away with entirely as it is checked inside the mutex - but since the mutex is global, that may not be the best way. Signed-off-by: David Howells Reported-by: Mateusz Guzik Signed-off-by: Andrew Morton Signed-off-by: James Morris Signed-off-by: Ben Hutchings --- security/keys/process_keys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -54,7 +54,7 @@ int install_user_keyrings(void) kenter("%p{%u}", user, user->uid); - if (user->uid_keyring) { + if (user->uid_keyring && user->session_keyring) { kleave(" = 0 [exist]"); return 0; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/