On Tue May 21, 2024 at 10:25 AM EEST, Bharat Bhushan wrote:
> > + rc = crypto_akcipher_encrypt(req);
> > + rc = crypto_wait_req(rc, &cwait);
> > +
>
> Few Minor comments,
> Extra line here
Yeah, makes sense.
> > + if (!rc)
> > + rc = req->dst_len;
> > +
> > + akcipher_request_free(req);
> > +
> > +err_tfm:
> > + crypto_free_akcipher(tfm);
> > +
> > + return rc;
> > +}
> > +
> > +static int __tpm2_key_rsa_decrypt(struct tpm_chip *chip,
> > + struct tpm2_key_rsa *key,
> > + struct kernel_pkey_params *params,
> > + const void *in, int in_len, void *out)
> > +{
> > + unsigned int offset = 0;
> > + u32 key_handle = 0;
> > + struct tpm_buf buf;
> > + u16 decrypted_len;
> > + u32 parent;
> > + u8 *pos;
> > + int ret;
> > +
> > + ret = tpm_try_get_ops(chip);
> > + if (ret)
> > + return ret;
> > +
> > + ret = tpm2_start_auth_session(chip);
> > + if (ret)
> > + goto err_ops;
> > +
> > + if (key->key.parent == TPM2_RH_NULL) {
> > + ret = tpm2_load_context(chip, chip->null_key_context,
> > &offset,
> > + &parent);
> > + if (ret) {
> > + ret = -EIO;
> > + goto err_auth;
> > + }
> > + } else {
> > + parent = key->key.parent;
> > + }
>
> Do we need braces here?
I think I added them because checkpatch complained me about not having
them. So I guess the rule is that if any branch has braces, all of them
should have...
>
> > +
> > + ret = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
> > + if (ret < 0)
> > + goto err_parent;
> > +
> > + tpm_buf_append_name(chip, &buf, parent, NULL);
> > + tpm_buf_append_hmac_session(chip, &buf,
> > TPM2_SA_CONTINUE_SESSION |
> > + TPM2_SA_ENCRYPT, NULL, 0);
> > + tpm_buf_append(&buf, key->key.blob, key->key.blob_len);
> > + if (buf.flags & TPM_BUF_OVERFLOW) {
> > + ret = -E2BIG;
> > + goto err_buf;
> > + }
> > + tpm_buf_fill_hmac_session(chip, &buf);
> > + ret = tpm_transmit_cmd(chip, &buf, 4, "RSA key loading");
> > + ret = tpm_buf_check_hmac_response(chip, &buf, ret);
> > + if (ret) {
> > + ret = -EIO;
> > + goto err_buf;
> > + }
> > + key_handle = be32_to_cpup((__be32
> > *)&buf.data[TPM_HEADER_SIZE]);
> > +
> > + tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_RSA_DECRYPT);
> > + tpm_buf_append_name(chip, &buf, key_handle, NULL);
> > + tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
> > NULL, 0);
> > + tpm_buf_append_u16(&buf, in_len);
> > + tpm_buf_append(&buf, in, in_len);
> > + tpm_buf_append_u16(&buf, TPM_ALG_NULL);
> > + tpm_buf_append_u16(&buf, 0);
> > + tpm_buf_fill_hmac_session(chip, &buf);
> > + ret = tpm_transmit_cmd(chip, &buf, 4, "RSA key decrypting");
> > + ret = tpm_buf_check_hmac_response(chip, &buf, ret);
> > + if (ret) {
> > + ret = -EIO;
> > + goto err_blob;
> > + }
> > +
> > + pos = buf.data + TPM_HEADER_SIZE + 4;
> > + decrypted_len = be16_to_cpup((__be16 *)pos);
> > + pos += 2;
> > +
> > + if (params->out_len < decrypted_len) {
> > + ret = -EMSGSIZE;
> > + goto err_blob;
> > + }
> > +
> > + memcpy(out, pos, decrypted_len);
> > + ret = decrypted_len;
> > +
> > +err_blob:
> > + tpm2_flush_context(chip, key_handle);
> > +
> > +err_buf:
> > + tpm_buf_destroy(&buf);
> > +
> > +err_parent:
> > + if (key->key.parent == TPM2_RH_NULL)
> > + tpm2_flush_context(chip, parent);
> > +
> > +err_auth:
> > + if (ret < 0)
> > + tpm2_end_auth_session(chip);
> > +
> > +err_ops:
> > + tpm_put_ops(chip);
> > + return ret;
> > +}
> > +
> > +static int tpm2_key_rsa_decrypt(struct tpm_chip *chip, struct tpm2_key_rsa
> > *key,
> > + struct kernel_pkey_params *params,
> > + const void *in, void *out)
> > +{
> > + const u8 *ptr;
> > + int out_len;
> > + u8 *work;
> > + int ret;
> > +
> > + work = kzalloc(params->out_len, GFP_KERNEL);
> > + if (!work)
> > + return -ENOMEM;
>
> Maybe use ERR_PTR() here and couple of more places
Hmm... but the function returns 'int'?
BR, Jarkko
