From cb5564e4b430ed587f73aae357ef4f3b15f4b2dc Mon Sep 17 00:00:00 2001 From: Adrian Stratulat Date: Wed, 30 Oct 2019 12:15:27 +0100 Subject: keys: CVE-2017-15299 KEYS: don't let add_key() update an uninstantiated key References: https://nvd.nist.gov/vuln/detail/CVE-2017-15299 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=60ff5b2f547af3828aebafd54daded44cfb0807a https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-4.9.y&id=da0c7503c0b886784bf8bcb279c7d71c1e50c438 Change-Id: Ia6933016fae4fa49769ef37c340978ccb9caa422 Signed-off-by: Adrian Stratulat --- patches/cve/CVE-2017-15299.patch | 126 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 patches/cve/CVE-2017-15299.patch diff --git a/patches/cve/CVE-2017-15299.patch b/patches/cve/CVE-2017-15299.patch new file mode 100644 index 0000000..8f60e35 --- /dev/null +++ b/patches/cve/CVE-2017-15299.patch @@ -0,0 +1,126 @@ +From da0c7503c0b886784bf8bcb279c7d71c1e50c438 Mon Sep 17 00:00:00 2001 +From: David Howells +Date: Thu, 12 Oct 2017 16:00:41 +0100 +Subject: KEYS: don't let add_key() update an uninstantiated key + +commit 60ff5b2f547af3828aebafd54daded44cfb0807a upstream. + +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 + #include + #include + + 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-g428490e38b2e #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 + +Upstream-Status: Backport [https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-4.9.y&id=da0c7503c0b886784bf8bcb279c7d71c1e50c438] +CVE: CVE-2017-15299 + +Reported-by: Eric Biggers +Signed-off-by: David Howells +cc: Eric Biggers +Signed-off-by: Greg Kroah-Hartman +Signed-off-by: Adrian Stratulat +--- + security/keys/key.c | 10 ++++++++++ + 1 file changed, 10 insertions(+) + +diff --git a/security/keys/key.c b/security/keys/key.c +index 135e1eb7e468..dd6dcee02492 100644 +--- a/security/keys/key.c ++++ b/security/keys/key.c +@@ -905,6 +905,16 @@ error: + */ + __key_link_end(keyring, &index_key, edit); + ++ key = key_ref_to_ptr(key_ref); ++ if (test_bit(KEY_FLAG_USER_CONSTRUCT, &key->flags)) { ++ ret = wait_for_key_construction(key, true); ++ if (ret < 0) { ++ key_ref_put(key_ref); ++ key_ref = ERR_PTR(ret); ++ goto error_free_prep; ++ } ++ } ++ + key_ref = __key_update(key_ref, &prep); + goto error_free_prep; + } +-- +cgit 1.2-0.3.lf.el7 + -- cgit v1.2.3-54-g00ecf