RE: RE: [PATCH v8 1/1] crypto: Introduce RSA algorithm

[Gonglei (Arei)]

> -----Original Message-----
> From: zhenwei pi [mailto:pizhenwei@bytedance.com]
> Sent: Tuesday, May 31, 2022 9:48 AM
> To: Gonglei (Arei) <arei.gonglei@huawei.com>
> Cc: qemu-devel@nongnu.org; mst@redhat.com;
> virtualization@lists.linux-foundation.org; helei.sig11@bytedance.com;
> berrange@redhat.com
> Subject: Re: RE: [PATCH v8 1/1] crypto: Introduce RSA algorithm
> 
> On 5/30/22 21:31, Gonglei (Arei) wrote:
> >
> >
> >> -----Original Message-----
> >> From: zhenwei pi [mailto:pizhenwei@bytedance.com]
> >> Sent: Friday, May 27, 2022 4:48 PM
> >> To: mst@redhat.com; Gonglei (Arei) <arei.gonglei@huawei.com>
> >> Cc: qemu-devel@nongnu.org; virtualization@lists.linux-foundation.org;
> >> helei.sig11@bytedance.com; berrange@redhat.com; zhenwei pi
> >> <pizhenwei@bytedance.com>
> >> Subject: [PATCH v8 1/1] crypto: Introduce RSA algorithm
> >>
> >>
> > Skip...
> >
> >> +static int64_t
> >> +virtio_crypto_create_asym_session(VirtIOCrypto *vcrypto,
> >> +               struct virtio_crypto_akcipher_create_session_req
> >> *sess_req,
> >> +               uint32_t queue_id, uint32_t opcode,
> >> +               struct iovec *iov, unsigned int out_num) {
> >> +    VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
> >> +    CryptoDevBackendSessionInfo info = {0};
> >> +    CryptoDevBackendAsymSessionInfo *asym_info;
> >> +    int64_t session_id;
> >> +    int queue_index;
> >> +    uint32_t algo, keytype, keylen;
> >> +    g_autofree uint8_t *key = NULL;
> >> +    Error *local_err = NULL;
> >> +
> >> +    algo = ldl_le_p(&sess_req->para.algo);
> >> +    keytype = ldl_le_p(&sess_req->para.keytype);
> >> +    keylen = ldl_le_p(&sess_req->para.keylen);
> >> +
> >> +    if ((keytype != VIRTIO_CRYPTO_AKCIPHER_KEY_TYPE_PUBLIC)
> >> +         && (keytype !=
> VIRTIO_CRYPTO_AKCIPHER_KEY_TYPE_PRIVATE)) {
> >> +        error_report("unsupported asym keytype: %d", keytype);
> >> +        return -VIRTIO_CRYPTO_NOTSUPP;
> >> +    }
> >> +
> >> +    if (keylen) {
> >> +        key = g_malloc(keylen);
> >> +        if (iov_to_buf(iov, out_num, 0, key, keylen) != keylen) {
> >> +            virtio_error(vdev, "virtio-crypto asym key incorrect");
> >> +            return -EFAULT;
> >
> > Memory leak.
> >
> >> +        }
> >> +        iov_discard_front(&iov, &out_num, keylen);
> >> +    }
> >> +
> >> +    info.op_code = opcode;
> >> +    asym_info = &info.u.asym_sess_info;
> >> +    asym_info->algo = algo;
> >> +    asym_info->keytype = keytype;
> >> +    asym_info->keylen = keylen;
> >> +    asym_info->key = key;
> >> +    switch (asym_info->algo) {
> >> +    case VIRTIO_CRYPTO_AKCIPHER_RSA:
> >> +        asym_info->u.rsa.padding_algo =
> >> +            ldl_le_p(&sess_req->para.u.rsa.padding_algo);
> >> +        asym_info->u.rsa.hash_algo =
> >> +            ldl_le_p(&sess_req->para.u.rsa.hash_algo);
> >> +        break;
> >> +
> >> +    /* TODO DSA&ECDSA handling */
> >> +
> >> +    default:
> >> +        return -VIRTIO_CRYPTO_ERR;
> >> +    }
> >> +
> >> +    queue_index = virtio_crypto_vq2q(queue_id);
> >> +    session_id =
> >> + cryptodev_backend_create_session(vcrypto->cryptodev,
> >> &info,
> >> +                     queue_index, &local_err);
> >> +    if (session_id < 0) {
> >> +        if (local_err) {
> >> +            error_report_err(local_err);
> >> +        }
> >> +        return -VIRTIO_CRYPTO_ERR;
> >> +    }
> >> +
> >> +    return session_id;
> >
> > Where to free the key at both normal and exceptional paths?
> >
> 
> Hi, Lei
> 
> The key is declared with g_autofree:
> g_autofree uint8_t *key = NULL;
> 

OK. For the patch:

Reviewed-by: Gonglei <arei.gonglei@huawei.com>


Regards,
-Gonglei