Re: [PATCH v2 3/3] tpm: Address !chip->auth in tpm_buf_append_hmac_session*()

["Jarkko Sakkinen" <jarkko@kernel.org>]

On Fri Jul 5, 2024 at 5:05 PM EEST, Stefan Berger wrote:
> The original thread here
>
> https://lore.kernel.org/linux-integrity/656b319fc58683e399323b880722434467cf20f2.camel@kernel.org/T/#t
>
> identified the fact that tpm2_session_init() was missing for the ibmvtpm 
> driver. It is a non-zero problem for the respective platforms where this 
> driver is being used. The patched fixed the reported issue.

All bugs needs to be fixed always before features are added. You are
free now to submit your change as a feature patch, which will be
reviewed and applied later on.

> Now that you fixed it in v4 are you going to accept my original patch 
> with the Fixes tag since we will (likely) have an enabled feature in 
> 6.10 that is not actually working when the ibmvtpm driver is being used?

There's no bug in tpm_ibmvtpm driver as it functions as well as in 6.9.

I can review it earliest in the week 31, as feature patch. This was my
holiday week, and I came back only to fix the bug in the authentication
session patch set.

> I do no think that this is true and its only tpm_ibmvtpm.c that need the 
> call to tpm2_session_init. All drivers that use TPM_OPS_AUTO_STARTUP 
> will run tpm_chip_register -> tpm_chip_bootstrap -> tpm_auto_startup -> 
> tpm2_auto_startup -> tpm2_sessions_init

Right my bad. I overlooked the call sites and you're correct in that
for anything with that flag on, it will be called.

It still changes nothing, as the commit you were pointing out in the
fixes tag does not implement initialization code, and we would not have
that flag in the first place, if it was mandatory [1].

[1] It could be that it is mandatory perhaps, but that is a different
story. Then we would render the whole flag out. I think this was anyway
good insight, even if by unintentionally, and we can reconsider removing
it some day.

BR, Jarkko