On Mon, 2019-12-09 at 10:09 +0000, David Woodhouse wrote:
> On Sat, 2019-12-07 at 21:11 -0800, James Bottomley wrote:
[...]
> > @@ -330,13 +332,16 @@ static int tpm2_load_cmd(struct tpm_chip
> > *chip,
> > unsigned int private_len;
> > unsigned int public_len;
> > unsigned int blob_len;
> > - u8 *blob;
> > + u8 *blob, *pub;
> > int rc;
> > + u32 attrs;
> >
> > rc = tpm2_key_decode(payload, options, &blob);
> > - if (rc)
> > + if (rc) {
> > /* old form */
> > blob = payload->blob;
> > + payload->old_format = 1;
> > + }
> >
> > /* new format carries keyhandle but old format doesn't */
> > if (!options->keyhandle)
> > @@ -347,6 +352,16 @@ static int tpm2_load_cmd(struct tpm_chip
> > *chip,
> > return -E2BIG;
> >
> > public_len = be16_to_cpup((__be16 *) &blob[2 +
> > private_len]);
> > +
> > + pub = blob + 2 + private_len + 2;
> > + /* key attributes are always at offset 4 */
> > + attrs = get_unaligned_be32(pub + 4);
>
>
> At this point I don't believe you've checked yet that payload-
> >blob_len is sufficient to know that these bytes exist.
Check added.
> I think you're reading 'private_len' from non-existent bytes too, if
> payload->blob_len is zero or one? Which I think was there before you
> started, but you touched it last...
Well, I started this because of bugs in the current code, so this is
just one more bug I have to fix.
James
