Re: [PATCH 2/6] tpm: add policy sessions

["Jarkko Sakkinen" <jarkko@kernel.org>]

On Tue Jul 16, 2024 at 5:08 PM EEST, Jarkko Sakkinen wrote:
> On Tue Jul 16, 2024 at 5:07 PM EEST, Jarkko Sakkinen wrote:
> > On Tue Jul 16, 2024 at 2:53 PM EEST, Jarkko Sakkinen wrote:
> > > > -	u8 name[AUTH_MAX_NAMES][2 + SHA512_DIGEST_SIZE];
> > > > +	u8 name[AUTH_MAX_NAMES][2 + HASH_MAX_DIGESTSIZE];
> >
> > Ouch, we definitely do not want 2-dimensional arrays. I missed this in
> > the hmac review.
> >
> > Why this is based on count (AUTH_MAX_NAMES) rather than space? Is that
> > value from the specs?
> >
> > You could just as well replace name and name_h with a single tpm_buf
> > instance in "sized" mode and return -E2BIG from the functions that use
> > it. Right, those don't return anything but void, which should be also
> > fixed.
>
> tpm_buf_write_u32()
> tpm_buf_write()
> tpm_buf_write_u32()
> tpm_buf_write()
>
> Two buffers stored. The read functions are non-destructive. Let's not
> invent ad-hoc crap when we have already a tested and legit tool for
> this.

Other issues that I saw is that the patch set does not apply anymore but
it is been two months so no wonder.

For the next version you should also specify a test transcript that
allows to test the functionality similarly as I've done for asymmetric
keys:

https://lore.kernel.org/linux-integrity/20240528210823.28798-1-jarkko@kernel.org/T/#mb07f85a8c3f4af388cbc08438e71ac8aea447d85

I don't want to invent the test case myself, and very few will do
I'd figure.

BR, Jarkko