From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jarkko Sakkinen Subject: Re: [PATCH 4/4] tpm: introduce tpm_extend_pcr_digests() Date: Fri, 7 Apr 2017 23:10:37 +0300 Message-ID: <20170407201037.sarb4mjgfj64hfhr@intel.com> References: <20170329102452.32212-1-roberto.sassu@huawei.com> <20170329102452.32212-5-roberto.sassu@huawei.com> <20170405121416.2rly5pizs2hll56k@intel.com> <259b67e8-216b-ad91-52c3-c4b39a8f3d1c@huawei.com> <88284005-3a53-1b37-e1f2-bfa88987c989@huawei.com> <20170407193156.thwubykqqleaszrt@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20170407193156.thwubykqqleaszrt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: tpmdd-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Roberto Sassu Cc: linux-ima-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: tpmdd-devel@lists.sourceforge.net On Fri, Apr 07, 2017 at 10:31:56PM +0300, Jarkko Sakkinen wrote: > On Fri, Apr 07, 2017 at 11:50:49AM +0200, Roberto Sassu wrote: > > On 4/5/2017 4:36 PM, Roberto Sassu wrote: > > > I have a question. As you can see in the IMA patch, I'm calling > > > tpm_is_tpm2() to determine if I should invoke tpm_pcr_extend(), > > > for TPM 1.2, or tpm_pcr_extend_digests(), for TPM 2.0. > > > > > > Should the new function work with TPM 1.2? If a tpm2_digest > > > structure with a SHA1 digest is provided, I could call > > > tpm_pcr_extend() instead of returning an error. > > > > Hi Jarkko > > > > would you have any objection if the new functions work > > regardless of the TPM version? > > > > Thanks > > > > Roberto > > Yes, you should not add multiple functions that do the same thing > essentially. Please rework tpm_pcr_extend instead. > > And while you are doing it, please also rework it to use tpm_buf > for everything. > > /Jarkko Some prework is required before you implement your new things. 1. tpm1_pcr_extend() to tpm-interface.c that is called by tpm_pcr_extend() and make it use tpm_buf. (1 commit) 2. There's a race condition bug in the way Nayna has implemented the digest list extension. It takes and releases tpm_mutex multiple times. This bug needs to be fixed before any other changes are justified (1 commit). Please add the Fixes line to the commit message. For (2) you should probably rename the existing tpm2_pcr_extend() as tpm2_pcr_extend_bank() and change it as a static function. That functio should take tpm_transmit flags as the last parameter. Then implement tpm2_pcr_extend() that does the same thing as is done now inside tpm_pcr_extend(). Call tpm2_pcr_extend_bank() inside that function with TPM_TRANSMIT_UNLOCKED. You should make this as its own patch set without any of the new additions. Only after these fixes are landed I'm ready to review any new extensions to tpm_pcr_extend(). PS I *purposely* have not read any of the IMA links that you have sent to me. You should be able to explain these changes without requiring such action. /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot