by Jarkko Sakkinen

[permalink] [raw]

Subject: Re: [PATCH v6 1/6] tpm: Open code tpm_buf_parameters()

On Tue May 28, 2024 at 6:51 AM EEST, Jarkko Sakkinen wrote:
> With only single call site, this makes zero sense (slipped out of the
> radar during the review). Open code and document the action directly
> to the site, to make it more readable.
>
> Fixes: 1b6d7f9eb150 ("tpm: add session encryption protection to tpm2_get_random()")
> Signed-off-by: Jarkko Sakkinen <[email protected]>
> ---
> drivers/char/tpm/tpm-buf.c | 26 --------------------------
> drivers/char/tpm/tpm2-cmd.c | 10 +++++++++-
> include/linux/tpm.h | 2 --
> 3 files changed, 9 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
> index 647c6ca92ac3..cad0048bcc3c 100644
> --- a/drivers/char/tpm/tpm-buf.c
> +++ b/drivers/char/tpm/tpm-buf.c
> @@ -223,30 +223,4 @@ u32 tpm_buf_read_u32(struct tpm_buf *buf, off_t *offset)
> }
> EXPORT_SYMBOL_GPL(tpm_buf_read_u32);
>
> -static u16 tpm_buf_tag(struct tpm_buf *buf)
> -{
> - struct tpm_header *head = (struct tpm_header *)buf->data;
> -
> - return be16_to_cpu(head->tag);
> -}
> -
> -/**
> - * tpm_buf_parameters - return the TPM response parameters area of the tpm_buf
> - * @buf: tpm_buf to use
> - *
> - * Where the parameters are located depends on the tag of a TPM
> - * command (it's immediately after the header for TPM_ST_NO_SESSIONS
> - * or 4 bytes after for TPM_ST_SESSIONS). Evaluate this and return a
> - * pointer to the first byte of the parameters area.
> - *
> - * @return: pointer to parameters area
> - */
> -u8 *tpm_buf_parameters(struct tpm_buf *buf)
> -{
> - int offset = TPM_HEADER_SIZE;
> -
> - if (tpm_buf_tag(buf) == TPM2_ST_SESSIONS)
> - offset += 4;
>
> - return &buf->data[offset];
> -}
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 0cdf892ec2a7..1e856259219e 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -281,6 +281,7 @@ struct tpm2_get_random_out {
> int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> {
> struct tpm2_get_random_out *out;
> + struct tpm_header *head;
> struct tpm_buf buf;
> u32 recd;
> u32 num_bytes = max;
> @@ -288,6 +289,7 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> int total = 0;
> int retries = 5;
> u8 *dest_ptr = dest;
> + off_t offset;
>
> if (!num_bytes || max > TPM_MAX_RNG_DATA)
> return -EINVAL;
> @@ -320,7 +322,13 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> goto out;
> }
>
> - out = (struct tpm2_get_random_out *)tpm_buf_parameters(&buf);
> + head = (struct tpm_header *)buf.data;
> + offset = TPM_HEADER_SIZE;
> + /* Skip the parameter size field: */
> + if (be16_to_cpu(head->tag) == TPM2_ST_SESSIONS)
> + offset += 4;
> +
> + out = (struct tpm2_get_random_out *)&buf.data[offset];
> recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
> if (tpm_buf_length(&buf) <
> TPM_HEADER_SIZE +
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index c17e4efbb2e5..b3217200df28 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -437,8 +437,6 @@ u8 tpm_buf_read_u8(struct tpm_buf *buf, off_t *offset);
> u16 tpm_buf_read_u16(struct tpm_buf *buf, off_t *offset);
> u32 tpm_buf_read_u32(struct tpm_buf *buf, off_t *offset);
>
> -u8 *tpm_buf_parameters(struct tpm_buf *buf);
> -
> /*
> * Check if TPM device is in the firmware upgrade mode.
> */

This patch went into v6 by mistake, unrelated to the patch set.

BR, Jarkko