> -----Original Message-----
> From: Antoine Tenart <antoine.tenart@xxxxxxxxxxx>
> Sent: Wednesday, September 11, 2019 5:30 PM
> To: Pascal van Leeuwen <pascalvanl@xxxxxxxxx>
> Cc: linux-crypto@xxxxxxxxxxxxxxx; antoine.tenart@xxxxxxxxxxx;
> herbert@xxxxxxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx; Pascal Van Leeuwen
> <pvanleeuwen@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH 2/2] crypto: inside-secure - Add support for the Chacha20-Poly1305
> AEAD
>
> Hello Pascal,
>
> On Tue, Sep 10, 2019 at 04:38:13PM +0200, Pascal van Leeuwen wrote:
> > @@ -43,8 +44,8 @@ struct safexcel_cipher_ctx {
> >
> > u32 mode;
> > enum safexcel_cipher_alg alg;
> > - bool aead;
> > - int xcm; /* 0=authenc, 1=GCM, 2 reserved for CCM */
> > + char aead; /* !=0=AEAD, 2=IPSec ESP AEAD */
> > + char xcm; /* 0=authenc, 1=GCM, 2 reserved for CCM */
>
> You could use an u8 instead. It also seems the aead comment has an
> issue, I'll let you check that.
>
Yes, u8 would be better, I'll fix that.
I don't see what's wrong with the comment though?
Anything unequal to 0 is AEAD, with value 2 being the ESP variant.
> > - dev_err(priv->dev, "aead: unsupported hash algorithm\n");
> > + dev_err(priv->dev, "aead: unsupported hash algorithmn");
>
> You remove the '\' here.
>
Oops. Must've accidentally happended during editing. Good catch.
> > @@ -440,6 +459,17 @@ static int safexcel_context_control(struct safexcel_cipher_ctx
> *ctx,
> > CONTEXT_CONTROL_DIGEST_XCM |
> > ctx->hash_alg |
> > CONTEXT_CONTROL_SIZE(ctrl_size);
> > + } else if (ctx->alg == SAFEXCEL_CHACHA20) {
> > + /* Chacha20-Poly1305 */
> > + cdesc->control_data.control0 =
> > + CONTEXT_CONTROL_KEY_EN |
> > + CONTEXT_CONTROL_CRYPTO_ALG_CHACHA20 |
> > + (sreq->direction == SAFEXCEL_ENCRYPT ?
> > + CONTEXT_CONTROL_TYPE_ENCRYPT_HASH_OUT :
> > + CONTEXT_CONTROL_TYPE_HASH_DECRYPT_IN) |
> > + ctx->hash_alg |
> > + CONTEXT_CONTROL_SIZE(ctrl_size);
>
> I think you could use an if + |= for readability here.
>
Ok, I can do that
> > +static int safexcel_aead_chachapoly_crypt(struct aead_request *req,
> > + enum safexcel_cipher_direction dir)
> > +{
> > + struct safexcel_cipher_req *creq = aead_request_ctx(req);
> > + struct crypto_aead *aead = crypto_aead_reqtfm(req);
> > + struct crypto_tfm *tfm = crypto_aead_tfm(aead);
> > + struct safexcel_cipher_ctx *ctx = crypto_tfm_ctx(tfm);
> > + struct aead_request *subreq = aead_request_ctx(req);
> > + u32 key[CHACHA_KEY_SIZE / sizeof(u32) + 1];
>
> Shouldn't you explicitly memzero the key at the end of the function?
>
Yes! :-) And good catch, again.
> Thanks!
> Antoine
>
> --
> Antoine Ténart, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Thanks,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com
