Re: [PATCH] tpm: ibmvtpm: Call tpm2_sessions_init() to initialize session support

[Stefan Berger <stefanb@linux.ibm.com>]

On 6/28/24 12:39, James Bottomley wrote:
> On Fri, 2024-06-28 at 10:54 +1000, Michael Ellerman wrote:
>> Stefan Berger <stefanb@linux.ibm.com> writes:
>>> Fix the following type of error message caused by a missing call to
>>> tpm2_sessions_init() in the IBM vTPM driver:
>>>
>>> [    2.987131] tpm tpm0: tpm2_load_context: failed with a TPM error
>>> 0x01C4
>>> [    2.987140] ima: Error Communicating to TPM chip, result: -14
>>>
>>> Fixes: d2add27cf2b8 ("tpm: Add NULL primary creation")
>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>>> ---
>>>   drivers/char/tpm/tpm_ibmvtpm.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c
>>> b/drivers/char/tpm/tpm_ibmvtpm.c
>>> index d3989b257f42..1e5b107d1f3b 100644
>>> --- a/drivers/char/tpm/tpm_ibmvtpm.c
>>> +++ b/drivers/char/tpm/tpm_ibmvtpm.c
>>> @@ -698,6 +698,10 @@ static int tpm_ibmvtpm_probe(struct vio_dev
>>> *vio_dev,
>>>                  rc = tpm2_get_cc_attrs_tbl(chip);
>>>                  if (rc)
>>>                          goto init_irq_cleanup;
>>> +
>>> +               rc = tpm2_sessions_init(chip);
>>> +               if (rc)
>>> +                       goto init_irq_cleanup;
>>>          }
>>>   
>>>          return tpm_chip_register(chip);
>>
>> #regzbot ^introduced: d2add27cf2b8
> 
> Could you please test out the patch I proposed for this:
> 
> https://lore.kernel.org/linux-integrity/1302b413a2d7bf3b275133e7fdb04b44bfe2d5e3.camel@HansenPartnership.com/
> 
> Because it's not just tmp_ibmvtpm that doesn't call autostart.  From
> inspection xen-tpmfront, tmp_nsc, tpm_infineon and tpm_atmel also

afaik tpm_infineon is a TPM 1.2 driver; same holds for tpm_atmel and 
tpm_ns. Neither needs this new call from what I understand. The new TPM2 
drivers have the TPM_OPS_AUTO_STARTUP flag set.

$ grep -r AUTO drivers/char/tpm/*.c | grep =
drivers/char/tpm/tpm_crb.c:     .flags = TPM_OPS_AUTO_STARTUP,
drivers/char/tpm/tpm_ftpm_tee.c:        .flags = TPM_OPS_AUTO_STARTUP,
drivers/char/tpm/tpm_i2c_atmel.c:       .flags = TPM_OPS_AUTO_STARTUP,
drivers/char/tpm/tpm_i2c_infineon.c:    .flags = TPM_OPS_AUTO_STARTUP,
drivers/char/tpm/tpm_i2c_nuvoton.c:     .flags = TPM_OPS_AUTO_STARTUP,
drivers/char/tpm/tpm_ibmvtpm.c: .flags = TPM_OPS_AUTO_STARTUP,
drivers/char/tpm/tpm_tis_core.c:        .flags = TPM_OPS_AUTO_STARTUP,
drivers/char/tpm/tpm_tis_i2c_cr50.c:    .flags = TPM_OPS_AUTO_STARTUP,
drivers/char/tpm/tpm_vtpm_proxy.c:      .flags = TPM_OPS_AUTO_STARTUP,

With xen-tpmfront I am not sure where something like chip->flags |= 
TPM_CHIP_FLAG_TPM2 is done -- tpm2-cmd.c::tpm2_probe is not called from 
this driver but only from tpm_tis_core.c::tpm_tis_core_init and 
otherwise driver set it themselves.

$ grep -r TPM_CHIP_FLAG_TPM2 drivers/char/tpm/*.c | grep =
drivers/char/tpm/tpm2-cmd.c:                    chip->flags |= 
TPM_CHIP_FLAG_TPM2;
drivers/char/tpm/tpm-chip.c:    rc = (chip->flags & TPM_CHIP_FLAG_TPM2) ?
drivers/char/tpm/tpm_crb.c:     chip->flags = TPM_CHIP_FLAG_TPM2;
drivers/char/tpm/tpm_ftpm_tee.c:        pvt_data->chip->flags |= 
TPM_CHIP_FLAG_TPM2;
drivers/char/tpm/tpm_i2c_nuvoton.c:             chip->flags |= 
TPM_CHIP_FLAG_TPM2;
drivers/char/tpm/tpm_ibmvtpm.c:         chip->flags |= TPM_CHIP_FLAG_TPM2;
drivers/char/tpm/tpm-interface.c:       rc = (chip->flags & 
TPM_CHIP_FLAG_TPM2) != 0;
drivers/char/tpm/tpm_tis_i2c_cr50.c:    chip->flags |= TPM_CHIP_FLAG_TPM2;
drivers/char/tpm/tpm_vtpm_proxy.c:              proxy_dev->chip->flags 
|= TPM_CHIP_FLAG_TPM2;





> don't, so it would be better to fix this for everyone rather than just
> for you and have to do a separate fix for each of them.

I am not sure whether any one of the mentioned drivers actually need 
this call and if they need it they should probably move towards setting 
TPM_OPS_AUTO_STARTUP.

   Stefan
> 
> James
> 
>