From patchwork Thu Mar 30 23:50:23 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mat Martineau X-Patchwork-Id: 9655311 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 5ACFC6034C for ; Thu, 30 Mar 2017 23:50:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4D29328682 for ; Thu, 30 Mar 2017 23:50:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 41F0F28683; Thu, 30 Mar 2017 23:50:45 +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.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,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 B277E2867E for ; Thu, 30 Mar 2017 23:50:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934702AbdC3Xum (ORCPT ); Thu, 30 Mar 2017 19:50:42 -0400 Received: from mga11.intel.com ([192.55.52.93]:25154 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934690AbdC3Xuf (ORCPT ); Thu, 30 Mar 2017 19:50:35 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1490917834; x=1522453834; h=from:to:cc:subject:date:message-id:in-reply-to: references; bh=AVTmU+fdmdxZJGwn8hAtJpVixLaOom4uEAodI/IK6rI=; b=KmUvruwGAt8gDyRhyDvPqMeU42Wh6/Xaiu3JIkhxspsfATBHK3d/lskO HlialQ+E7pb0N6RnnlR4sthpJurtUA==; Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 Mar 2017 16:50:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,249,1486454400"; d="scan'208";a="82507821" Received: from mjmartin-nuc02.sea.intel.com ([10.252.134.39]) by fmsmga005.fm.intel.com with ESMTP; 30 Mar 2017 16:50:30 -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 v13 06/10] KEYS: Consistent ordering for __key_link_begin and restrict check Date: Thu, 30 Mar 2017 16:50:23 -0700 Message-Id: <20170330235027.6879-7-mathew.j.martineau@linux.intel.com> X-Mailer: git-send-email 2.12.2 In-Reply-To: <20170330235027.6879-1-mathew.j.martineau@linux.intel.com> References: <20170330235027.6879-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 405a6e1c62d6..14512f5760d4 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->key); 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); @@ -855,21 +857,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->key); 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);