linux-kernel - RE: [PATCH v6 03/20] tpm: factor out tpm 1.x duration calculation to tpm1-cmd.c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5B8DA87D05A7694D9FA63FD143655C1B9D9F49D7@hasmsx109.ger.corp.intel.com>
Date:   Wed, 17 Oct 2018 15:28:27 +0000
From:   "Winkler, Tomas" <tomas.winkler@...el.com>
To:     Nayna Jain <nayna@...ux.ibm.com>
CC:     Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        Jason Gunthorpe <jgg@...pe.ca>,
        Nayna Jain <nayna@...ux.vnet.ibm.com>,
        "Usyskin, Alexander" <alexander.usyskin@...el.com>,
        "Struk, Tadeusz" <tadeusz.struk@...el.com>,
        "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
        "linux-security-module@...r.kernel.org" 
        <linux-security-module@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Mimi Zohar <zohar@...ux.ibm.com>
Subject: RE: [PATCH v6 03/20] tpm: factor out tpm 1.x duration calculation
 to tpm1-cmd.c


> 
> 
> On 10/17/2018 05:54 PM, Winkler, Tomas wrote:
> >>
> >>>    	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> >>> -	rc = i2c_nuvoton_wait_for_data_avail(chip,
> >>> -					     tpm_calc_ordinal_duration(chip,
> >>> -								       ordinal),
> >>> -					     &priv->read_queue);
> >>> +	duration = tpm1_calc_ordinal_duration(chip, ordinal);
> >>
> >> This version of the patch didn't address my previous comment - "The
> >> original code in the nuvoton driver does not differentiate between
> >> TPM 1.2 and TPM
> >> 2.0 as it does in tpm_tis_core.c.
> >> Before making any changes, I would first fix it, so that it could
> >> easily be backported. Only then do the refactoring."
> > This patch doesn't change the original behavior, just change the name of
> the function,  so there is no regression.
> > I would suggest there is another bug in those drivers/devices that is
> orthogonal to this refactoring and should not block this from merging.
> 
> The problem is that you are inadvertently fixing a bug without realizing it -
> [Patch 04/20]. Bug fixes should be addressed independently of this change,
> so that they can be backported properly.
>
This would be ideal,  but that's happen more than often that the fix cannot be applied w/o changes to older kernels.
I can send that patch, are you able to test it? Frankly nobody complained about it so I'm not sure what to do.
 
> > According to what you say  it can call just
> > tpm_calc_oridnal_duration() instead of tpm1_calc_ordinal_duration(chip,
> ordinal), but I prefer that someone that has those devices will do that
> change on top of this series as  I cannot test it.
> 
> The problem is:
> 
> 1. This patch calls tpm1_calc_ordinal_duration for both the TPM 1.2 and
> TPM 2.0.
That's correct 
> 2. In the next patch, it adds a new function tpm_calc_ordinal_duration as a
> wrapper for both the TPM 1.2 and TPM 2.0. After this change when it calls
> tpm_calc_ordinal_duration(), it now calls different functions for TPM 1.2 and
> TPM 2.0. This is a change in behavior.
That's not correct,   I missed that.  Either someone can test the TPM 2.0 device  or I need to revert it for nuvoton, in the next patch.  

Thanks
Tomas