On Fri, Apr 07, 2017 at 11:10:37PM +0300, Jarkko Sakkinen wrote: > 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
And there was one big problem in your first patches you did not have the RFC tag and still you did not include the kernel mailing list. I won't apply or give reviewed-by to any patches that do not linux-kernel and for non-trivial changes by defacto also include linux-security-module. /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
