linux-kernel - [GIT][KEYS] fix race with concurrent install_user

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Tue, 12 Mar 2013 17:09:06 +1100 (EST)
From:	James Morris <jmorris@...ei.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
cc:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: [GIT][KEYS] fix race with concurrent install_user_keyrings()

Please apply.


The following changes since commit 7c6baa304b841673d3a55ea4fcf9a5cbf7a1674b:

  Merge branch 'perf-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2013-03-11 07:54:29 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus

David Howells (1):
      keys: fix race with concurrent install_user_keyrings()

 security/keys/process_keys.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

commit 0da9dfdd2cd9889201bc6f6f43580c99165cd087
Author: David Howells <dhowells@...hat.com>
Date:   Tue Mar 12 16:44:31 2013 +1100

    keys: fix race with concurrent install_user_keyrings()
    
    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 <dhowells@...hat.com>
    Reported-by: Mateusz Guzik <mguzik@...hat.com>
    Cc: <stable@...nel.org>
    Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
    Signed-off-by: James Morris <james.l.morris@...cle.com>

diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index a571fad..42defae 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -57,7 +57,7 @@ int install_user_keyrings(void)
 
 	kenter("%p{%u}", 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/