• David Howells's avatar
    KEYS: don't let add_key() update an uninstantiated key · 60ff5b2f
    David Howells authored
    Currently, when passed a key that already exists, add_key() will call the
    key's ->update() method if such exists.  But this is heavily broken in the
    case where the key is uninstantiated because it doesn't call
    __key_instantiate_and_link().  Consequently, it doesn't do most of the
    things that are supposed to happen when the key is instantiated, such as
    setting the instantiation state, clearing KEY_FLAG_USER_CONSTRUCT and
    awakening tasks waiting on it, and incrementing key->user->nikeys.
    
    It also never takes key_construction_mutex, which means that
    ->instantiate() can run concurrently with ->update() on the same key.  In
    the case of the "user" and "logon" key types this causes a memory leak, at
    best.  Maybe even worse, the ->update() methods of the "encrypted" and
    "trusted" key types actually just dereference a NULL pointer when passed an
    uninstantiated key.
    
    Change key_create_or_update() to wait interruptibly for the key to finish
    construction before continuing.
    
    This patch only affects *uninstantiated* keys.  For now we still allow a
    negatively instantiated key to be updated (thereby positively
    instantiating it), although that's broken too (the next patch fixes it)
    and I'm not sure that anyone actually uses that functionality either.
    
    Here is a simple reproducer for the bug using the "encrypted" key type
    (requires CONFIG_ENCRYPTED_KEYS=y), though as noted above the bug
    pertained to more than just the "encrypted" key type:
    
        #include <stdlib.h>
        #include <unistd.h>
        #include <keyutils.h>
    
        int main(void)
        {
            int ringid = keyctl_join_session_keyring(NULL);
    
            if (fork()) {
                for (;;) {
                    const char payload[] = "update user:foo 32";
    
                    usleep(rand() % 10000);
                    add_key("encrypted", "desc", payload, sizeof(payload), ringid);
                    keyctl_clear(ringid);
                }
            } else {
                for (;;)
                    request_key("encrypted", "desc", "callout_info", ringid);
            }
        }
    
    It causes:
    
        BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
        IP: encrypted_update+0xb0/0x170
        PGD 7a178067 P4D 7a178067 PUD 77269067 PMD 0
        PREEMPT SMP
        CPU: 0 PID: 340 Comm: reproduce Tainted: G      D         4.14.0-rc1-00025-g428490e3 #796
        Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
        task: ffff8a467a39a340 task.stack: ffffb15c40770000
        RIP: 0010:encrypted_update+0xb0/0x170
        RSP: 0018:ffffb15c40773de8 EFLAGS: 00010246
        RAX: 0000000000000000 RBX: ffff8a467a275b00 RCX: 0000000000000000
        RDX: 0000000000000005 RSI: ffff8a467a275b14 RDI: ffffffffb742f303
        RBP: ffffb15c40773e20 R08: 0000000000000000 R09: ffff8a467a275b17
        R10: 0000000000000020 R11: 0000000000000000 R12: 0000000000000000
        R13: 0000000000000000 R14: ffff8a4677057180 R15: ffff8a467a275b0f
        FS:  00007f5d7fb08700(0000) GS:ffff8a467f200000(0000) knlGS:0000000000000000
        CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
        CR2: 0000000000000018 CR3: 0000000077262005 CR4: 00000000001606f0
        Call Trace:
         key_create_or_update+0x2bc/0x460
         SyS_add_key+0x10c/0x1d0
         entry_SYSCALL_64_fastpath+0x1f/0xbe
        RIP: 0033:0x7f5d7f211259
        RSP: 002b:00007ffed03904c8 EFLAGS: 00000246 ORIG_RAX: 00000000000000f8
        RAX: ffffffffffffffda RBX: 000000003b2a7955 RCX: 00007f5d7f211259
        RDX: 00000000004009e4 RSI: 00000000004009ff RDI: 0000000000400a04
        RBP: 0000000068db8bad R08: 000000003b2a7955 R09: 0000000000000004
        R10: 000000000000001a R11: 0000000000000246 R12: 0000000000400868
        R13: 00007ffed03905d0 R14: 0000000000000000 R15: 0000000000000000
        Code: 77 28 e8 64 34 1f 00 45 31 c0 31 c9 48 8d 55 c8 48 89 df 48 8d 75 d0 e8 ff f9 ff ff 85 c0 41 89 c4 0f 88 84 00 00 00 4c 8b 7d c8 <49> 8b 75 18 4c 89 ff e8 24 f8 ff ff 85 c0 41 89 c4 78 6d 49 8b
        RIP: encrypted_update+0xb0/0x170 RSP: ffffb15c40773de8
        CR2: 0000000000000018
    
    Cc: <stable@vger.kernel.org> # v2.6.12+
    Reported-by: default avatarEric Biggers <ebiggers@google.com>
    Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
    cc: Eric Biggers <ebiggers@google.com>
    60ff5b2f
key.c 30.7 KB