[lkml]   [2013]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    Subject[PATCH] 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

    =============================== ===============================
    ==>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.

    Reported-by: Mateusz Guzik <>
    Signed-off-by: David Howells <>

    security/keys/process_keys.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
    index 58dfe08..c5ec083 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;

     \ /
      Last update: 2013-03-07 00:21    [W:0.024 / U:120.940 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site