From patchwork Sat Apr 1 22:36:45 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 9658323 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5ACBC60353 for ; Sat, 1 Apr 2017 22:39:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 42B592852B for ; Sat, 1 Apr 2017 22:39:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 36DF128552; Sat, 1 Apr 2017 22:39:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B2EC32852B for ; Sat, 1 Apr 2017 22:39:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751993AbdDAWjx (ORCPT ); Sat, 1 Apr 2017 18:39:53 -0400 Received: from mail-pg0-f68.google.com ([74.125.83.68]:35735 "EHLO mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751992AbdDAWjw (ORCPT ); Sat, 1 Apr 2017 18:39:52 -0400 Received: by mail-pg0-f68.google.com with SMTP id g2so23084093pge.2; Sat, 01 Apr 2017 15:39:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=pUhU59BbOf/0bwvvluaWVbyIKtEKj0XtXGEwlG0Tb18=; b=es3Yamf0sUNd/aOULhgT/hj14PxHP1F4S80L/YC7gMCzgzxZbqVmCxSldaZgmDvZYx EdOi4Wn2/rOlTmIJiTBHknaIXdp5al9m/22gQhDNEJeAxJriLg/WF/Z5vSu0pr7MKb6J T6D51JdNFaSrXHhbhVA+/azLVCykqyGBJs09l7LsBIlxvTDIrm+vScYNd5BeftPhocF4 +OkEOqmqRrGos0p3UQqF2uBUZD96D8FIWON7z1E4rh8+TPxpxHem9DIG6dx3sIScfopw 8Eg4lrt0Bccu+fiL3ikREKADw8fF0Vcp1f+ogARVvgyfgB6WjMcPHkz39/SV2vxRwvSf Z7VA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=pUhU59BbOf/0bwvvluaWVbyIKtEKj0XtXGEwlG0Tb18=; b=p5IjHqBFgZUz2cP8hLtJ54pHsY3a2dt+AVZ4iN+VlpQk7mKo7b4RqOuBODUUCuNrmK G8iejwha85rz6BOfGzjO02AuznkIJ0BqZO3pQUjyW/pq0pUp0AeCcTNgMT7iK4qF8mDa 2N3iUsEtrALIKm+mJ7NPwin3tYGiLJDcT5GKWD+fOnpyAtctlbMdzL7hHbIY8OKfpxDP +BZqTg7hBYK1JEU4y1RI6Ggiogao5rSZiEaduEcRFOhzSFmRvwRpyd8zDIBhXOmn0DZ1 uuCrZuxpS4NJLI7IKROLcB/19cY1xA5P6nV6AR2OMdLV8/npG9gHsFX5o12+twLzCmzZ 4sag== X-Gm-Message-State: AFeK/H39Ix2JLUr/OBBsWsobycdlsrE5HfkHsepMpeGE232LwyB+W9SXs4WxuydDqOorVQ== X-Received: by 10.99.52.202 with SMTP id b193mr9679454pga.131.1491086391304; Sat, 01 Apr 2017 15:39:51 -0700 (PDT) Received: from zzz.hsd1.wa.comcast.net (c-73-239-167-150.hsd1.wa.comcast.net. [73.239.167.150]) by smtp.gmail.com with ESMTPSA id 123sm17718160pgh.21.2017.04.01.15.39.50 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 01 Apr 2017 15:39:50 -0700 (PDT) From: Eric Biggers To: keyrings@vger.kernel.org Cc: David Howells , linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, Eric Biggers , stable@vger.kernel.org Subject: [PATCH] KEYS: fix freeing uninitialized memory in key_update() Date: Sat, 1 Apr 2017 15:36:45 -0700 Message-Id: <20170401223645.29528-1-ebiggers3@gmail.com> X-Mailer: git-send-email 2.12.1 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP From: Eric Biggers key_update() freed the key_preparsed_payload even if it was not initialized first. This would cause a crash if userspace called keyctl_update() on a key with type like "asymmetric" that has a ->preparse() method but not an ->update() method. Possibly it could even be triggered for other key types by racing with keyctl_setperm() to make the KEY_NEED_WRITE check fail (the permission was already checked, so normally it wouldn't fail there). Reproducer with key type "asymmetric", given a valid cert.der: keyctl new_session keyid=$(keyctl padd asymmetric desc @s < cert.der) keyctl setperm $keyid 0x3f000000 keyctl update $keyid data [ 150.686666] BUG: unable to handle kernel NULL pointer dereference at 0000000000000001 [ 150.687601] IP: asymmetric_key_free_kids+0x12/0x30 [ 150.688139] PGD 38a3d067 [ 150.688141] PUD 3b3de067 [ 150.688447] PMD 0 [ 150.688745] [ 150.689160] Oops: 0000 [#1] SMP [ 150.689455] Modules linked in: [ 150.689769] CPU: 1 PID: 2478 Comm: keyctl Not tainted 4.11.0-rc4-xfstests-00187-ga9f6b6b8cd2f #742 [ 150.690916] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-20170228_101828-anatol 04/01/2014 [ 150.692199] task: ffff88003b30c480 task.stack: ffffc90000350000 [ 150.692952] RIP: 0010:asymmetric_key_free_kids+0x12/0x30 [ 150.693556] RSP: 0018:ffffc90000353e58 EFLAGS: 00010202 [ 150.694142] RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000000004 [ 150.694845] RDX: ffffffff81ee3920 RSI: ffff88003d4b0700 RDI: 0000000000000001 [ 150.697569] RBP: ffffc90000353e60 R08: ffff88003d5d2140 R09: 0000000000000000 [ 150.702483] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001 [ 150.707393] R13: 0000000000000004 R14: ffff880038a4d2d8 R15: 000000000040411f [ 150.709720] FS: 00007fcbcee35700(0000) GS:ffff88003fd00000(0000) knlGS:0000000000000000 [ 150.711504] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 150.712733] CR2: 0000000000000001 CR3: 0000000039eab000 CR4: 00000000003406e0 [ 150.714487] Call Trace: [ 150.714975] asymmetric_key_free_preparse+0x2f/0x40 [ 150.715907] key_update+0xf7/0x140 [ 150.716560] ? key_default_cmp+0x20/0x20 [ 150.717319] keyctl_update_key+0xb0/0xe0 [ 150.718066] SyS_keyctl+0x109/0x130 [ 150.718663] entry_SYSCALL_64_fastpath+0x1f/0xc2 [ 150.719440] RIP: 0033:0x7fcbce75ff19 [ 150.719926] RSP: 002b:00007ffd5d167088 EFLAGS: 00000206 ORIG_RAX: 00000000000000fa [ 150.720918] RAX: ffffffffffffffda RBX: 0000000000404d80 RCX: 00007fcbce75ff19 [ 150.721874] RDX: 00007ffd5d16785e RSI: 000000002866cd36 RDI: 0000000000000002 [ 150.722827] RBP: 0000000000000006 R08: 000000002866cd36 R09: 00007ffd5d16785e [ 150.723781] R10: 0000000000000004 R11: 0000000000000206 R12: 0000000000404d80 [ 150.724650] R13: 00007ffd5d16784d R14: 00007ffd5d167238 R15: 000000000040411f [ 150.725447] Code: 83 c4 08 31 c0 5b 41 5c 41 5d 41 5e 41 5f 5d c3 66 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 85 ff 74 23 55 48 89 e5 53 48 89 fb <48> 8b 3f e8 06 21 c5 ff 48 8b 7b 08 e8 fd 20 c5 ff 48 89 df e8 [ 150.727489] RIP: asymmetric_key_free_kids+0x12/0x30 RSP: ffffc90000353e58 [ 150.728117] CR2: 0000000000000001 [ 150.728430] ---[ end trace f7f8fe1da2d5ae8d ]--- Fixes: 4d8c0250b841 ("KEYS: Call ->free_preparse() even after ->preparse() returns an error") Cc: stable@vger.kernel.org # 3.17+ Signed-off-by: Eric Biggers --- security/keys/key.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/security/keys/key.c b/security/keys/key.c index 346fbf201c22..2f4ce35ae2aa 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -962,12 +962,11 @@ int key_update(key_ref_t key_ref, const void *payload, size_t plen) /* the key must be writable */ ret = key_permission(key_ref, KEY_NEED_WRITE); if (ret < 0) - goto error; + return ret; /* attempt to update it if supported */ - ret = -EOPNOTSUPP; if (!key->type->update) - goto error; + return -EOPNOTSUPP; memset(&prep, 0, sizeof(prep)); prep.data = payload;