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

[Jarkko Sakkinen]

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 <jarkko@xxxxxxxxxx>
> ---
>  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