From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, Eric Biggers <ebiggers@google.com>,
David Howells <dhowells@redhat.com>
Subject: [PATCH 4.13 64/85] KEYS: dont let add_key() update an uninstantiated key
Date: Tue, 24 Oct 2017 15:07:38 +0200 [thread overview]
Message-ID: <20171024125656.568379109@linuxfoundation.org> (raw)
In-Reply-To: <20171024125654.028122623@linuxfoundation.org>
4.13-stable review patch. If anyone has any objections, please let me know.
------------------
From: David Howells <dhowells@redhat.com>
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 <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-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
Reported-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Eric Biggers <ebiggers@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
security/keys/key.c | 10 ++++++++++
1 file changed, 10 insertions(+)
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -945,6 +945,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;
}
next prev parent reply other threads:[~2017-10-24 13:16 UTC|newest]
Thread overview: 88+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-24 13:06 [PATCH 4.13 00/85] 4.13.10-stable review Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 01/85] staging: bcm2835-audio: Fix memory corruption Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 02/85] USB: devio: Revert "USB: devio: Dont corrupt user memory" Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 03/85] USB: core: fix out-of-bounds access bug in usb_get_bos_descriptor() Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 04/85] USB: serial: metro-usb: add MS7820 device id Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 05/85] usb: cdc_acm: Add quirk for Elatec TWN3 Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 07/85] usb: hub: Allow reset retry for USB2 devices on connect bounce Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 08/85] ALSA: usb-audio: Add native DSD support for Pro-Ject Pre Box S2 Digital Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 09/85] can: gs_usb: fix busy loop if no more TX context is available Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 10/85] scsi: qla2xxx: Fix uninitialized work element Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 11/85] nbd: dont set the device size until were connected Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 12/85] s390/cputime: fix guest/irq/softirq times after CPU hotplug Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 13/85] parisc: Fix double-word compare and exchange in LWS code on 32-bit kernels Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 14/85] parisc: Fix detection of nonsynchronous cr16 cycle counters Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 15/85] iio: dummy: events: Add missing break Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 16/85] usb: musb: sunxi: Explicitly release USB PHY on exit Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 17/85] USB: musb: fix session-bit runtime-PM quirk Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 18/85] USB: musb: fix late external abort on suspend Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 19/85] usb: musb: musb_cppi41: Fix the address of teardown and autoreq registers Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 20/85] usb: musb: musb_cppi41: Fix cppi41_set_dma_mode() for DA8xx Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 21/85] usb: musb: musb_cppi41: Configure the number of channels " Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 22/85] usb: musb: Check for host-mode using is_host_active() on reset interrupt Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 23/85] xhci: Identify USB 3.1 capable hosts by their port protocol capability Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 24/85] xhci: Cleanup current_cmd in xhci_cleanup_command_queue() Greg Kroah-Hartman
2017-10-24 13:06 ` [PATCH 4.13 25/85] usb: xhci: Reset halted endpoint if trb is noop Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 26/85] usb: xhci: Handle error condition in xhci_stop_device() Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 28/85] can: af_can: can_pernet_init(): add missing error handling for kzalloc returning NULL Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 29/85] can: flexcan: fix state transition regression Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 30/85] can: flexcan: rename legacy error state quirk Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 31/85] can: flexcan: implement error passive " Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 32/85] can: flexcan: fix i.MX6 state transition issue Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 33/85] can: flexcan: fix i.MX28 " Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 34/85] can: flexcan: fix p1010 " Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 35/85] KEYS: encrypted: fix dereference of NULL user_key_payload Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 36/85] mmc: sdhci-pci: Fix default d3_retune for Intel host controllers Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 38/85] drm/nouveau/kms/nv50: fix oops during DP IRQ handling on non-MST boards Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 39/85] drm/nouveau/bsp/g92: disable by default Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 40/85] drm/nouveau/mmu: flush tlbs before deleting page tables Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 41/85] media: s5p-cec: add NACK detection support Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 42/85] media: cec: Respond to unregistered initiators, when applicable Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 43/85] media: dvb: i2c transfers over usb cannot be done from stack Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 44/85] tracing/samples: Fix creation and deletion of simple_thread_fn creation Greg Kroah-Hartman
2017-10-30 19:57 ` Steven Rostedt
2017-10-30 21:17 ` Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 45/85] ALSA: seq: Enable use locking in all configurations Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 46/85] ALSA: hda: Remove superfluous - added by printk conversion Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 47/85] ALSA: hda: Abort capability probe at invalid register read Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 48/85] i2c: ismt: Separate I2C block read from SMBus block read Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 50/85] Revert "tools/power turbostat: stop migrating, unless -m" Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 51/85] Input: stmfts - fix setting ABS_MT_POSITION_* maximum size Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 52/85] brcmfmac: Add check for short event packets Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 53/85] brcmsmac: make some local variables static const to reduce stack size Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 54/85] ARM: dts: sun6i: Fix endpoint IDs in second display pipeline Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 55/85] bus: mbus: fix window size calculation for 4GB windows Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 56/85] clockevents/drivers/cs5535: Improve resilience to spurious interrupts Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 57/85] rtlwifi: rtl8821ae: Fix connection lost problem Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 58/85] x86/microcode/intel: Disable late loading on model 79 Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 59/85] lib/digsig: fix dereference of NULL user_key_payload Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 60/85] fscrypt: " Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 61/85] ecryptfs: " Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 62/85] KEYS: Fix race between updating and finding a negative key Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 63/85] FS-Cache: fix dereference of NULL user_key_payload Greg Kroah-Hartman
2017-10-24 13:07 ` Greg Kroah-Hartman [this message]
2017-10-24 13:07 ` [PATCH 4.13 65/85] pkcs7: Prevent NULL pointer dereference, since sinfo is not always set Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 66/85] arm64: dts: rockchip: correct vqmmc voltage for rk3399 platforms Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 67/85] ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 68/85] iomap_dio_rw: Allocate AIO completion queue before submitting dio Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 69/85] xfs: dont unconditionally clear the reflink flag on zero-block files Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 70/85] xfs: evict CoW fork extents when performing finsert/fcollapse Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 71/85] fs/xfs: Use %pS printk format for direct addresses Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 72/85] xfs: report zeroed or not correctly in xfs_zero_range() Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 73/85] xfs: update i_size after unwritten conversion in dio completion Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 74/85] xfs: perag initialization should only touch m_ag_max_usable for AG 0 Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 75/85] xfs: Capture state of the right inode in xfs_iflush_done Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 76/85] xfs: always swap the cow forks when swapping extents Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 77/85] xfs: handle racy AIO in xfs_reflink_end_cow Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 78/85] xfs: Dont log uninitialised fields in inode structures Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 79/85] xfs: move more RT specific code under CONFIG_XFS_RT Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 80/85] xfs: dont change inode mode if ACL update fails Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 81/85] xfs: reinit btree pointer on attr tree inactivation walk Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 82/85] xfs: handle error if xfs_btree_get_bufs fails Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 83/85] xfs: cancel dirty pages on invalidation Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 84/85] xfs: trim writepage mapping to within eof Greg Kroah-Hartman
2017-10-24 13:07 ` [PATCH 4.13 85/85] xfs: move two more RT specific functions into CONFIG_XFS_RT Greg Kroah-Hartman
2017-10-25 21:09 ` Arnd Bergmann
2017-10-26 7:09 ` Greg Kroah-Hartman
2017-10-24 21:28 ` [PATCH 4.13 00/85] 4.13.10-stable review Guenter Roeck
2017-10-25 6:48 ` Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171024125656.568379109@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=dhowells@redhat.com \
--cc=ebiggers@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).