Hi Stephan,

> -----Original Message-----
> From: Stephan Mueller [mailto:smuel...@chronox.de]
> Sent: Saturday, March 19, 2016 6:59 PM
> To: Tudor-Dan Ambarus
> Cc: herb...@gondor.apana.org.au; tadeusz.st...@intel.com; linux-
> cry...@vger.kernel.org; Horia Ioan Geanta Neag
> Subject: Re: [PATCH 10/10] crypto: caam - add support for RSA algorithm
> 
> > +void rsa_free_key(struct rsa_raw_key *key)
> > +{
> > +   kzfree(key->d);
> > +   key->d = NULL;
> > +
> > +   kfree(key->e);
> > +   key->e = NULL;
> > +
> > +   kfree(key->n);
> > +   key->n = NULL;
> > +
> > +   key->n_sz = 0;
> > +   key->e_sz = 0;
> > +}
> 
> As you implement raw key support for use in other drivers, shouldn't that
> function go into some helper file like free_mpis().
> > +

[ta] I should also add an rsa_free_coherent_key(struct device *dev, struct 
rsa_raw_key *key), for those implementations that use the DMA-coherent API. 


> > +static int caam_rsa_dec(struct akcipher_request *req)
> > +{
> > +   struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
> > +   struct rsa_raw_ctx *ctx = akcipher_tfm_ctx(tfm);
> > +   struct rsa_raw_key *key = &ctx->key;
> > +   struct device *jrdev = ctx->dev;
> > +   struct rsa_edesc *edesc = NULL;
> > +   size_t desclen = sizeof(struct rsa_priv_f1_desc);
> > +   int ret;
> > +
> > +   if (unlikely(!key->n || !key->d))
> > +           return -EINVAL;
> > +
> > +   if (req->dst_len < key->n_sz) {
> > +           req->dst_len = key->n_sz;
> > +           return -EOVERFLOW;
> > +   }
> > +
> > +   /* Allocate extended descriptor. */
> > +   edesc = rsa_edesc_alloc(req, desclen);
> > +   if (IS_ERR(edesc))
> > +           return PTR_ERR(edesc);
> > +
> > +   /* Initialize Job Descriptor. */
> > +   ret = init_rsa_priv_f1_desc(req, edesc);
> > +   if (ret)
> > +           return ret;
> 
> Same here, kfree?

[ta] Sure, thanks.

> > +
> > +   ret = caam_jr_enqueue(jrdev, edesc->hw_desc, rsa_priv_f1_done, req);
> > +   if (!ret) {
> > +           ret = -EINPROGRESS;
> 
> dto
>

[ta] resources are freed on rsa_priv_f1_done callback.
 
> > +static struct akcipher_alg rsa = {
> 
> can you please name that something else, like caam_rsa? It is a real hassle
> when searching some symbol and I get such a generic name.

[ta] ok.

Thank you for the review!
ta
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to