On Wed, Nov 22, 2023 at 12:31:20AM +0200, Jarkko Sakkinen wrote: > Take advantage of the new sized buffer (TPM2B) mode of struct tpm_buf in > tpm2_seal_trusted(). This allows to add robustness to the command > construction without requiring to calculate buffer sizes manually. > > Signed-off-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx> > --- > v3 [2023-11-21]: A boundary error check as response for the feeedback > from Mario Limenciello: > https://lore.kernel.org/linux-integrity/3f9086f6-935f-48a7-889b-c71398422fa1@xxxxxxx/ > v2: Use tpm_buf_read_* > --- > security/keys/trusted-keys/trusted_tpm2.c | 54 +++++++++++++---------- > 1 file changed, 31 insertions(+), 23 deletions(-) > > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c > index bc700f85f80b..97b1dfca2dba 100644 > --- a/security/keys/trusted-keys/trusted_tpm2.c > +++ b/security/keys/trusted-keys/trusted_tpm2.c > @@ -228,8 +228,9 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > struct trusted_key_payload *payload, > struct trusted_key_options *options) > { > + off_t offset = TPM_HEADER_SIZE; > + struct tpm_buf buf, sized; > int blob_len = 0; > - struct tpm_buf buf; > u32 hash; > u32 flags; > int i; > @@ -258,6 +259,14 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > return rc; > } > > + rc = tpm_buf_init_sized(&sized); > + if (rc) { > + tpm_buf_destroy(&buf); It won't really hurt, but at the moment if tpm_buf_init_sized() returns non-zero, then it must be returning -ENOMEM, and tpm_buf_destroy(&buf) is not needed, right? > + tpm_put_ops(chip); > + return rc; > + } > + > + tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE); > tpm_buf_append_u32(&buf, options->keyhandle); > tpm2_buf_append_auth(&buf, TPM2_RS_PW, > NULL /* nonce */, 0, > @@ -266,36 +275,36 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > TPM_DIGEST_SIZE); > > /* sensitive */ > - tpm_buf_append_u16(&buf, 4 + options->blobauth_len + payload->key_len); > + tpm_buf_append_u16(&sized, options->blobauth_len); > > - tpm_buf_append_u16(&buf, options->blobauth_len); > if (options->blobauth_len) > - tpm_buf_append(&buf, options->blobauth, options->blobauth_len); > + tpm_buf_append(&sized, options->blobauth, options->blobauth_len); > > - tpm_buf_append_u16(&buf, payload->key_len); > - tpm_buf_append(&buf, payload->key, payload->key_len); > + tpm_buf_append_u16(&sized, payload->key_len); > + tpm_buf_append(&sized, payload->key, payload->key_len); > + tpm_buf_append(&buf, sized.data, sized.length); > > /* public */ > - tpm_buf_append_u16(&buf, 14 + options->policydigest_len); > - tpm_buf_append_u16(&buf, TPM_ALG_KEYEDHASH); > - tpm_buf_append_u16(&buf, hash); > + tpm_buf_reset_sized(&sized); > + tpm_buf_append_u16(&sized, TPM_ALG_KEYEDHASH); > + tpm_buf_append_u16(&sized, hash); > > /* key properties */ > flags = 0; > flags |= options->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH; > - flags |= payload->migratable ? 0 : (TPM2_OA_FIXED_TPM | > - TPM2_OA_FIXED_PARENT); > - tpm_buf_append_u32(&buf, flags); > + flags |= payload->migratable ? 0 : (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT); > + tpm_buf_append_u32(&sized, flags); > > /* policy */ > - tpm_buf_append_u16(&buf, options->policydigest_len); > + tpm_buf_append_u16(&sized, options->policydigest_len); > if (options->policydigest_len) > - tpm_buf_append(&buf, options->policydigest, > - options->policydigest_len); > + tpm_buf_append(&sized, options->policydigest, options->policydigest_len); > > /* public parameters */ > - tpm_buf_append_u16(&buf, TPM_ALG_NULL); > - tpm_buf_append_u16(&buf, 0); > + tpm_buf_append_u16(&sized, TPM_ALG_NULL); > + tpm_buf_append_u16(&sized, 0); > + > + tpm_buf_append(&buf, sized.data, sized.length); > > /* outside info */ > tpm_buf_append_u16(&buf, 0); > @@ -312,21 +321,20 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > if (rc) > goto out; > > - blob_len = be32_to_cpup((__be32 *) &buf.data[TPM_HEADER_SIZE]); > - if (blob_len > MAX_BLOB_SIZE) { > + blob_len = tpm_buf_read_u32(&buf, &offset); > + if (blob_len > MAX_BLOB_SIZE || buf.flags & TPM_BUF_BOUNDARY_ERROR) { > rc = -E2BIG; > goto out; > } > - if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) { > + if (buf.length - offset < blob_len) { > rc = -EFAULT; > goto out; > } > > - blob_len = tpm2_key_encode(payload, options, > - &buf.data[TPM_HEADER_SIZE + 4], > - blob_len); > + blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len); > > out: > + tpm_buf_destroy(&sized); > tpm_buf_destroy(&buf); > > if (rc > 0) { > -- > 2.42.1
