From patchwork Tue Nov 29 00:44:31 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mat Martineau X-Patchwork-Id: 9450819 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 5FACB600CB for ; Tue, 29 Nov 2016 00:44:47 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 563BA275A2 for ; Tue, 29 Nov 2016 00:44:47 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4AF1F27D64; Tue, 29 Nov 2016 00:44:47 +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 005AD275A2 for ; Tue, 29 Nov 2016 00:44:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755225AbcK2Aoq (ORCPT ); Mon, 28 Nov 2016 19:44:46 -0500 Received: from mga09.intel.com ([134.134.136.24]:3114 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755047AbcK2Aol (ORCPT ); Mon, 28 Nov 2016 19:44:41 -0500 Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga102.jf.intel.com with ESMTP; 28 Nov 2016 16:44:38 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,565,1473145200"; d="scan'208";a="35292292" Received: from mjmartin-nuc01.wa.intel.com ([10.232.97.135]) by orsmga004.jf.intel.com with ESMTP; 28 Nov 2016 16:44:37 -0800 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 v10 10/11] KEYS: Consistent ordering for __key_link_begin and restrict check Date: Mon, 28 Nov 2016 16:44:31 -0800 Message-Id: <20161129004432.17926-11-mathew.j.martineau@linux.intel.com> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20161129004432.17926-1-mathew.j.martineau@linux.intel.com> References: <20161129004432.17926-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 keyring restrict callback was sometimes called before __key_link_begin and sometimes after, which meant that the keyring semaphores were not always held during the restrict callback. If the semaphores are consistently acquired before checking link restrictions, keyring contents cannot be changed after the restrict check is complete but before the evaluated key is linked to the keyring. Signed-off-by: Mat Martineau --- security/keys/key.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/security/keys/key.c b/security/keys/key.c index 441c512..e95c63c 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -500,21 +500,23 @@ int key_instantiate_and_link(struct key *key, } if (keyring) { + ret = __key_link_begin(keyring, &key->index_key, &edit); + if (ret < 0) + goto error; + if (keyring->restrict_link && keyring->restrict_link->check) { struct key_restriction *keyres = keyring->restrict_link; ret = keyres->check(keyring, key->type, &prep.payload, keyres->data); if (ret < 0) - goto error; + goto error_link_end; } - ret = __key_link_begin(keyring, &key->index_key, &edit); - if (ret < 0) - goto error; } ret = __key_instantiate_and_link(key, &prep, keyring, authkey, &edit); +error_link_end: if (keyring) __key_link_end(keyring, &key->index_key, edit); @@ -860,21 +862,21 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref, } index_key.desc_len = strlen(index_key.description); + ret = __key_link_begin(keyring, &index_key, &edit); + if (ret < 0) { + key_ref = ERR_PTR(ret); + goto error_free_prep; + } + if (restrict_link && restrict_link->check) { ret = restrict_link->check(keyring, index_key.type, &prep.payload, restrict_link->data); if (ret < 0) { key_ref = ERR_PTR(ret); - goto error_free_prep; + goto error_link_end; } } - ret = __key_link_begin(keyring, &index_key, &edit); - if (ret < 0) { - key_ref = ERR_PTR(ret); - goto error_free_prep; - } - /* if we're going to allocate a new key, we're going to have * to modify the keyring */ ret = key_permission(keyring_ref, KEY_NEED_WRITE);