From patchwork Mon Sep 26 23:36:34 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mat Martineau X-Patchwork-Id: 9351375 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 A62326077A for ; Mon, 26 Sep 2016 23:36:52 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9801628581 for ; Mon, 26 Sep 2016 23:36:52 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8C6D328EAD; Mon, 26 Sep 2016 23:36:52 +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.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI 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 C121628581 for ; Mon, 26 Sep 2016 23:36:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753766AbcIZXgv (ORCPT ); Mon, 26 Sep 2016 19:36:51 -0400 Received: from mga06.intel.com ([134.134.136.31]:35323 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753844AbcIZXgr (ORCPT ); Mon, 26 Sep 2016 19:36:47 -0400 Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP; 26 Sep 2016 16:36:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,401,1470726000"; d="scan'208";a="884066060" Received: from mjmartin-nuc01.wa.intel.com ([10.232.97.135]) by orsmga003.jf.intel.com with ESMTP; 26 Sep 2016 16:36:44 -0700 From: Mat Martineau To: keyrings@vger.kernel.org, linux-security-module@vger.kernel.org, dhowells@redhat.com Cc: Mat Martineau , zohar@linux.vnet.ibm.com Subject: [PATCH v7 1/9] KEYS: Add a rwsem to each key type Date: Mon, 26 Sep 2016 16:36:34 -0700 Message-Id: <20160926233642.32204-2-mathew.j.martineau@linux.intel.com> X-Mailer: git-send-email 2.10.0 In-Reply-To: <20160926233642.32204-1-mathew.j.martineau@linux.intel.com> References: <20160926233642.32204-1-mathew.j.martineau@linux.intel.com> Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP The key_types_sem read-write semaphore has been performing two jobs, protecting the key_types_list and making sure individual key types are not unregistered while they are in use. It is held for most of key_create_or_update, which means a second key type cannot be looked up by name while a key is being created or updated without re-acquiring the semaphore and triggering a lockdep warning: [ 445.752215] ============================================= [ 445.752946] [ INFO: possible recursive locking detected ] [ 445.753745] 4.8.0-rc1+ #34 Not tainted [ 445.754087] --------------------------------------------- [ 445.754540] test-key/948 is trying to acquire lock: [ 445.754950] (key_types_sem){++++.+}, at: [] key_type_lookup+0x1b/0x80 [ 445.755854] [ 445.755854] but task is already holding lock: [ 445.756515] (key_types_sem){++++.+}, at: [] key_type_lookup+0x1b/0x80 By adding a rwsem to struct key_type, key_types_sem only needs to be held when key_types_list is being accessed. key_type_put() is modified to only release the given key_type (the key_type parameter to this function was previously ignored). Signed-off-by: Mat Martineau --- include/linux/key-type.h | 1 + security/keys/gc.c | 2 +- security/keys/key.c | 32 +++++++++++++++++++++++--------- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/include/linux/key-type.h b/include/linux/key-type.h index 63b2a92..cfde86a 100644 --- a/include/linux/key-type.h +++ b/include/linux/key-type.h @@ -161,6 +161,7 @@ struct key_type { /* internal fields */ struct list_head link; /* link in types list */ struct lock_class_key lock_class; /* key->sem lock class */ + struct rw_semaphore sem; /* for safe key type removal */ }; extern struct key_type key_type_keyring; diff --git a/security/keys/gc.c b/security/keys/gc.c index addf060..467af2c 100644 --- a/security/keys/gc.c +++ b/security/keys/gc.c @@ -99,7 +99,7 @@ static void key_gc_timer_func(unsigned long data) * collect dead links and the third to clean up the dead keys. We have to be * careful as there may already be a cycle in progress. * - * The caller must be holding key_types_sem. + * The caller must be holding key_types_sem and ktype->sem. */ void key_gc_keytype(struct key_type *ktype) { diff --git a/security/keys/key.c b/security/keys/key.c index 346fbf2..c3556e5 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -672,8 +672,8 @@ error: /* * Find and lock the specified key type against removal. * - * We return with the sem read-locked if successful. If the type wasn't - * available -ENOKEY is returned instead. + * We return with the key type's sem read-locked if successful. If the + * type wasn't available -ENOKEY is returned instead. */ struct key_type *key_type_lookup(const char *type) { @@ -684,14 +684,18 @@ struct key_type *key_type_lookup(const char *type) /* look up the key type to see if it's one of the registered kernel * types */ list_for_each_entry(ktype, &key_types_list, link) { - if (strcmp(ktype->name, type) == 0) + /* ktype->sem is only write locked when unregistering, so + * ignore a key type with a contended lock. + */ + if ((strcmp(ktype->name, type) == 0) && + down_read_trylock(&ktype->sem)) goto found_kernel_type; } - up_read(&key_types_sem); ktype = ERR_PTR(-ENOKEY); found_kernel_type: + up_read(&key_types_sem); return ktype; } @@ -720,7 +724,8 @@ EXPORT_SYMBOL_GPL(key_set_timeout); */ void key_type_put(struct key_type *ktype) { - up_read(&key_types_sem); + if (ktype) + up_read(&ktype->sem); } /* @@ -1102,6 +1107,7 @@ int register_key_type(struct key_type *ktype) int ret; memset(&ktype->lock_class, 0, sizeof(ktype->lock_class)); + init_rwsem(&ktype->sem); ret = -EEXIST; down_write(&key_types_sem); @@ -1135,14 +1141,22 @@ EXPORT_SYMBOL(register_key_type); void unregister_key_type(struct key_type *ktype) { down_write(&key_types_sem); + down_write(&ktype->sem); list_del_init(&ktype->link); downgrade_write(&key_types_sem); key_gc_keytype(ktype); pr_notice("Key type %s unregistered\n", ktype->name); + up_write(&ktype->sem); up_read(&key_types_sem); } EXPORT_SYMBOL(unregister_key_type); +static void add_special_key_type(struct key_type *ktype) +{ + init_rwsem(&ktype->sem); + list_add_tail(&ktype->link, &key_types_list); +} + /* * Initialise the key management state. */ @@ -1153,10 +1167,10 @@ void __init key_init(void) 0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL); /* add the special key types */ - list_add_tail(&key_type_keyring.link, &key_types_list); - list_add_tail(&key_type_dead.link, &key_types_list); - list_add_tail(&key_type_user.link, &key_types_list); - list_add_tail(&key_type_logon.link, &key_types_list); + add_special_key_type(&key_type_keyring); + add_special_key_type(&key_type_dead); + add_special_key_type(&key_type_user); + add_special_key_type(&key_type_logon); /* record the root user tracking */ rb_link_node(&root_key_user.node,