Re: [PATCH v5 05/11] security: keys: trusted: Allow storage of PCR values in creation data

[Eric Biggers]

On Fri, Nov 11, 2022 at 03:16:30PM -0800, Evan Green wrote:
> +       creationpcrs= hex integer representing the set of PCRs to be
> +                     included in the creation data. For each bit set, the
> +                     corresponding PCR will be included in the key creation
> +                     data. Bit 0 corresponds to PCR0. Currently only the first
> +                     PC standard 24 PCRs are supported on the currently active
> +                     bank. Leading zeroes are optional. TPM2 only.

What does "currently active bank" mean?

> +		/* PCR bitmask */
> +		for (i = 0; i < 3; i++) {
> +			char tmp = 0;
> +
> +			for (j = 0; j < 8; j++) {
> +				char bit = (i * 8) + j;
> +
> +				if (options->creation_pcrs & (1 << bit))
> +					tmp |= (1 << j);
> +			}
> +			tpm_buf_append_u8(&buf, tmp);
> +		}

Why not just:

	tpm_buf_append_u8(&buf, options->creation_pcrs);
	tpm_buf_append_u8(&buf, options->creation_pcrs >> 8);
	tpm_buf_append_u8(&buf, options->creation_pcrs >> 16);

Also what if bit 24 or above is set?  Should an error be returned?

- Eric