From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752694AbbJOTWJ (ORCPT ); Thu, 15 Oct 2015 15:22:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54329 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751850AbbJOTWH convert rfc822-to-8bit (ORCPT ); Thu, 15 Oct 2015 15:22:07 -0400 Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 From: David Howells In-Reply-To: References: To: Dmitry Vyukov Cc: dhowells@redhat.com, james.l.morris@oracle.com, serge@hallyn.com, keyrings@vger.kernel.org, linux-security-module@vger.kernel.org, LKML , syzkaller@googlegroups.com, Kostya Serebryany , Alexander Potapenko , Andrey Konovalov , Sasha Levin , Julien Tinnes , Kees Cook Subject: Re: GPF in keyring_destroy MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <4764.1444936919.1@warthog.procyon.org.uk> Content-Transfer-Encoding: 8BIT Date: Thu, 15 Oct 2015 20:21:59 +0100 Message-ID: <4765.1444936919@warthog.procyon.org.uk> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Does the attached patch fix it for you? David --- commit a7609e0bb3973d6ee3c9f1ecd0b6a382d99d6248 Author: David Howells Date: Thu Oct 15 17:21:37 2015 +0100 KEYS: Fix crash when attempt to garbage collect an uninstantiated keyring The following sequence of commands: i=`keyctl add user a a @s` keyctl request2 keyring foo bar @t keyctl unlink $i @s tries to invoke an upcall to instantiate a keyring if one doesn't already exist by that name within the user's keyring set. However, if the upcall fails, the code sets keyring->type_data.reject_error to -ENOKEY or some other error code. When the key is garbage collected, the key destroy function is called unconditionally and keyring_destroy() uses list_empty() on keyring->type_data.link - which is in a union with reject_error. Subsequently, the kernel tries to unlink the keyring from the keyring names list - which oopses like this: BUG: unable to handle kernel paging request at 00000000ffffff8a IP: [] keyring_destroy+0x3d/0x88 ... Workqueue: events key_garbage_collector ... RIP: 0010:[] keyring_destroy+0x3d/0x88 RSP: 0018:ffff88003e2f3d30 EFLAGS: 00010203 RAX: 00000000ffffff82 RBX: ffff88003bf1a900 RCX: 0000000000000000 RDX: 0000000000000000 RSI: 000000003bfc6901 RDI: ffffffff81a73a40 RBP: ffff88003e2f3d38 R08: 0000000000000152 R09: 0000000000000000 R10: ffff88003e2f3c18 R11: 000000000000865b R12: ffff88003bf1a900 R13: 0000000000000000 R14: ffff88003bf1a908 R15: ffff88003e2f4000 ... CR2: 00000000ffffff8a CR3: 000000003e3ec000 CR4: 00000000000006f0 ... Call Trace: [] key_gc_unused_keys.constprop.1+0x5d/0x10f [] key_garbage_collector+0x1fa/0x351 [] process_one_work+0x28e/0x547 [] worker_thread+0x26e/0x361 [] ? rescuer_thread+0x2a8/0x2a8 [] kthread+0xf3/0xfb [] ? kthread_create_on_node+0x1c2/0x1c2 [] ret_from_fork+0x3f/0x70 [] ? kthread_create_on_node+0x1c2/0x1c2 Note the value in RAX. This is a 32-bit representation of -ENOKEY. The solution is to only call ->destroy() if the key was successfully instantiated. Reported-by: Dmitry Vyukov Signed-off-by: David Howells diff --git a/security/keys/gc.c b/security/keys/gc.c index 39eac1fd5706..addf060399e0 100644 --- a/security/keys/gc.c +++ b/security/keys/gc.c @@ -134,8 +134,10 @@ static noinline void key_gc_unused_keys(struct list_head *keys) kdebug("- %u", key->serial); key_check(key); - /* Throw away the key data */ - if (key->type->destroy) + /* Throw away the key data if the key is instantiated */ + if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags) && + !test_bit(KEY_FLAG_NEGATIVE, &key->flags) && + key->type->destroy) key->type->destroy(key); security_key_free(key);