On Fri, Nov 11, 2022 at 03:16:26PM -0800, Evan Green wrote:
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 1621ce8187052c..886277b2654e3b 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -342,6 +342,53 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> }
> EXPORT_SYMBOL_GPL(tpm_pcr_extend);
>
> +/**
> + * tpm2_pcr_reset - Reset the specified PCR
Should this function be in drivers/char/tpm/tpm2-cmd.c instead of here?
> + * @chip: A &struct tpm_chip instance, %NULL for the default chip
> + * @pcr_idx: The PCR to be reset
> + *
> + * Return: Same as with tpm_transmit_cmd(), or ENOTTY for TPM1 devices.
> + */
> +int tpm2_pcr_reset(struct tpm_chip *chip, u32 pcr_idx)
The callers of this function assume it returns a negative errno value. But
actually it can return positive TPM2_RC_* error codes as well. Probably you
should make it only return negative errno values.
> +{
> + struct tpm2_null_auth_area auth_area;
> + struct tpm_buf buf;
> + int rc;
> +
> + chip = tpm_find_get_ops(chip);
> + if (!chip)
> + return -ENODEV;
> +
> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> + rc = -ENOTTY;
> + goto out;
> + }
> +
> + rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_RESET);
> + if (rc)
> + goto out;
> +
> + tpm_buf_append_u32(&buf, pcr_idx);
> +
> + auth_area.handle = cpu_to_be32(TPM2_RS_PW);
> + auth_area.nonce_size = 0;
> + auth_area.attributes = 0;
> + auth_area.auth_size = 0;
> +
> + tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area));
sizeof(struct tpm2_null_auth_area) => sizeof(auth_area)
> + tpm_buf_append(&buf, (const unsigned char *)&auth_area,
unsigned char => u8
Also, since the code to append a "null" authorization area appears in both
tpm2_pcr_reset() and tpm2_pcr_extend(), perhaps it should be refactored into a
helper function?
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index dfeb25a0362dee..70134e6551745f 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -219,6 +219,7 @@ enum tpm2_command_codes {
> TPM2_CC_HIERARCHY_CONTROL = 0x0121,
> TPM2_CC_HIERARCHY_CHANGE_AUTH = 0x0129,
> TPM2_CC_CREATE_PRIMARY = 0x0131,
> + TPM2_CC_PCR_RESET = 0x013D,
> TPM2_CC_SEQUENCE_COMPLETE = 0x013E,
> TPM2_CC_SELF_TEST = 0x0143,
> TPM2_CC_STARTUP = 0x0144,
> @@ -293,6 +294,13 @@ struct tpm_header {
> };
> } __packed;
>
> +struct tpm2_null_auth_area {
> + __be32 handle;
> + __be16 nonce_size;
> + u8 attributes;
> + __be16 auth_size;
> +} __packed;
struct tpm2_null_auth_area is only used by code in drivers/char/tpm/, so should
its declaration go in the internal header drivers/char/tpm/tpm.h instead?
- Eric
