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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux