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

Reply via email to