From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751604AbcGUJC7 (ORCPT ); Thu, 21 Jul 2016 05:02:59 -0400 Received: from mga03.intel.com ([134.134.136.65]:63337 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751028AbcGUJC4 (ORCPT ); Thu, 21 Jul 2016 05:02:56 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,398,1464678000"; d="scan'208";a="850463769" Date: Thu, 21 Jul 2016 12:02:45 +0300 From: Jarkko Sakkinen To: Jason Gunthorpe Cc: Peter Huewe , linux-security-module@vger.kernel.org, Marcel Selhorst , "moderated list:TPM DEVICE DRIVER" , open list Subject: Re: [PATCH] tpm: fix a race condition tpm2_unseal_trusted() Message-ID: <20160721090245.GA7999@intel.com> References: <1468973792-17598-1-git-send-email-jarkko.sakkinen@linux.intel.com> <20160720164818.GA21460@obsidianresearch.com> <20160720205314.GA6525@intel.com> <20160720211332.GA32417@obsidianresearch.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160720211332.GA32417@obsidianresearch.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 20, 2016 at 03:13:32PM -0600, Jason Gunthorpe wrote: > On Wed, Jul 20, 2016 at 11:53:14PM +0300, Jarkko Sakkinen wrote: > > > The only use cases I see at the moment for it work this way: > > > > 1. Call tpm_try_get_ops. > > 2. Send a TPM command. > > 3. Call tpm_put_ops. > > Right, but that is just a reflection of what the in kernel users are > doing today, not necessarily what they should be doing. > > We should not break the put/get semantics.. > > > I did not find any other form of use. The only use is to make sure that > > there are no transactions running before the ops are cleared. Or did I > > overlook something perhaps? > > The put/get is intended to allow a kapi user to hold a ref to tpm > without it geting destroyed. It is not intended to be an exclusive lock. These operations *are not* exposed to kapi. They are interal to the driver. That's why it does not make sense speak about kapi user. All the places where it's used it's always used in the way that you also take the exclusive lock. You are speaking how it could be used. I'm looking at how it's actually used. Shouldn't implementation reflect that rather than future prospects? > > Trusted key unseal operation with TPM2 is broken into two operations: > > > > 1. Load the given key blob. > > 2. Unseal the data. > > > > Without locking and unlocking mutex only once there is a race condition. > > Well, the race condition is fundamentally because we don't have key > virtualization in the kernel :| I do agree and I'll share my thought about TPM2 context swapping in LSS 2016. > Those sorts of compound ops should hold the tpm_mutex manually, not > through the get_ops scheme. The next best option would be to have unlocked transmit_cmd function. /Jarkko From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jarkko Sakkinen Subject: Re: [PATCH] tpm: fix a race condition tpm2_unseal_trusted() Date: Thu, 21 Jul 2016 12:02:45 +0300 Message-ID: <20160721090245.GA7999@intel.com> References: <1468973792-17598-1-git-send-email-jarkko.sakkinen@linux.intel.com> <20160720164818.GA21460@obsidianresearch.com> <20160720205314.GA6525@intel.com> <20160720211332.GA32417@obsidianresearch.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20160720211332.GA32417-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: tpmdd-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Jason Gunthorpe Cc: "moderated list:TPM DEVICE DRIVER" , linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, open list List-Id: tpmdd-devel@lists.sourceforge.net On Wed, Jul 20, 2016 at 03:13:32PM -0600, Jason Gunthorpe wrote: > On Wed, Jul 20, 2016 at 11:53:14PM +0300, Jarkko Sakkinen wrote: > > > The only use cases I see at the moment for it work this way: > > > > 1. Call tpm_try_get_ops. > > 2. Send a TPM command. > > 3. Call tpm_put_ops. > > Right, but that is just a reflection of what the in kernel users are > doing today, not necessarily what they should be doing. > > We should not break the put/get semantics.. > > > I did not find any other form of use. The only use is to make sure that > > there are no transactions running before the ops are cleared. Or did I > > overlook something perhaps? > > The put/get is intended to allow a kapi user to hold a ref to tpm > without it geting destroyed. It is not intended to be an exclusive lock. These operations *are not* exposed to kapi. They are interal to the driver. That's why it does not make sense speak about kapi user. All the places where it's used it's always used in the way that you also take the exclusive lock. You are speaking how it could be used. I'm looking at how it's actually used. Shouldn't implementation reflect that rather than future prospects? > > Trusted key unseal operation with TPM2 is broken into two operations: > > > > 1. Load the given key blob. > > 2. Unseal the data. > > > > Without locking and unlocking mutex only once there is a race condition. > > Well, the race condition is fundamentally because we don't have key > virtualization in the kernel :| I do agree and I'll share my thought about TPM2 context swapping in LSS 2016. > Those sorts of compound ops should hold the tpm_mutex manually, not > through the get_ops scheme. The next best option would be to have unlocked transmit_cmd function. /Jarkko ------------------------------------------------------------------------------ What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports.http://sdm.link/zohodev2dev