Skip to content
  1. Mar 12, 2013
    • David Howells's avatar
      keys: fix race with concurrent install_user_keyrings() · 0da9dfdd
      David Howells authored
      
      
      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: default avatarDavid Howells <dhowells@redhat.com>
      Reported-by: default avatarMateusz Guzik <mguzik@redhat.com>
      Cc: <stable@kernel.org>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarJames Morris <james.l.morris@oracle.com>
      0da9dfdd
  2. Mar 11, 2013
  3. Mar 10, 2013
  4. Mar 09, 2013