On 7/4/24 02:41, Jarkko Sakkinen wrote:
On Thu Jul 4, 2024 at 4:56 AM EEST, Stefan Berger wrote:
On 7/3/24 14:24, Jarkko Sakkinen wrote:
Unless tpm_chip_bootstrap() was called by the driver, !chip->auth can
Doesn't tpm_chip_register() need to be called by all drivers? This
function then calls tpm_chip_bootstrap().
cause a null derefence in tpm_buf_hmac_session*(). Thus, address
!chip->auth in tpm_buf_hmac_session*() and remove the fallback
implementation for !TCG_TPM2_HMAC.
Cc: stable@xxxxxxxxxxxxxxx # v6.9+
Reported-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
Closes: https://lore.kernel.org/linux-integrity/20240617193408.1234365-1-stefanb@xxxxxxxxxxxxx/
Fixes: 1085b8276bb4 ("tpm: Add the rest of the session HMAC API")
Signed-off-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
I applied this series now but it doesn't solve the reported problem. The
It fixes the issues of which symptoms was shown by your transcript:
[ 2.987131] tpm tpm0: tpm2_load_context: failed with a TPM error 0x01C4
[ 2.987140] ima: Error Communicating to TPM chip, result: -14
Your original thread identified zero problems, so thus your claim here
is plain untrue.
The original thread here
https://lore.kernel.org/linux-integrity/656b319fc58683e399323b880722434467cf20f2.camel@xxxxxxxxxx/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.
Before the null derefence is fixed all other patches related are
blocked, including ibm_tpmvtpm patches, because it would be insane
to accept them when there is known memory corruption bug, which
this patch set fixes.
What is so difficult to understand in this?
error message is gone but the feature can still be enabled
(CONFIG_TCG_TPM2_HMAC=y) but is unlikely actually doing what it is
promising to do with this config option. So you either still have to
apply my patch, James's patch, or your intended "depends on
!TCG_IBMVTPM" patch.
Well this somewhat misleading imho...
None of the previous patches, including your, do nothing to fix the null
derefence bug and that is the *only* bug we care about ATM. With these
fixes drivers that do not call tpm_chip_bootstrap() will be fully
working still but without encryption.
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?
Original patch:
https://lore.kernel.org/linux-integrity/656b319fc58683e399323b880722434467cf20f2.camel@xxxxxxxxxx/T/#t
There's five drivers which would require update for that:
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_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;
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
$ grep AUTO_START *.c
tpm_crb.c: .flags = TPM_OPS_AUTO_STARTUP,
tpm_ftpm_tee.c: .flags = TPM_OPS_AUTO_STARTUP,
tpm_i2c_atmel.c: .flags = TPM_OPS_AUTO_STARTUP,
tpm_i2c_infineon.c: .flags = TPM_OPS_AUTO_STARTUP,
tpm_i2c_nuvoton.c: .flags = TPM_OPS_AUTO_STARTUP,
tpm-interface.c: if (!(chip->ops->flags & TPM_OPS_AUTO_STARTUP))
tpm_tis_core.c: .flags = TPM_OPS_AUTO_STARTUP,
tpm_tis_i2c_cr50.c: .flags = TPM_OPS_AUTO_STARTUP,
tpm_vtpm_proxy.c: .flags = TPM_OPS_AUTO_STARTUP,
All the above drivers are also calling tpm_chip_register.
tpm_atmel.c: rc = tpm_chip_register(chip);
tpm-chip.c: * tpm_chip_register() - create a character device for the
TPM chip
tpm-chip.c:int tpm_chip_register(struct tpm_chip *chip)
tpm-chip.c:EXPORT_SYMBOL_GPL(tpm_chip_register);
tpm-chip.c: * cleans up all the resources reserved by tpm_chip_register().
tpm_crb.c: rc = tpm_chip_register(chip);
tpm_ftpm_tee.c: rc = tpm_chip_register(pvt_data->chip);
tpm_ftpm_tee.c: dev_err(dev, "%s: tpm_chip_register failed with
rc=%d\n",
tpm_i2c_atmel.c: return tpm_chip_register(chip);
tpm_i2c_infineon.c: return tpm_chip_register(chip);
tpm_i2c_nuvoton.c: return tpm_chip_register(chip);
tpm_ibmvtpm.c: return tpm_chip_register(chip);
tpm_infineon.c: rc = tpm_chip_register(chip);
tpm_nsc.c: rc = tpm_chip_register(chip);
tpm_tis_core.c: rc = tpm_chip_register(chip);
tpm_tis_i2c_cr50.c: return tpm_chip_register(chip);
tpm_vtpm_proxy.c: rc = tpm_chip_register(proxy_dev->chip);
xen-tpmfront.c: return tpm_chip_register(priv->chip)
Stefan
BR, Jarkko
