Re: [PATCH 6/6] Add support for AEAD algos.

[Harsh Jain]

On 08-11-2016 19:51, Harsh Jain wrote:
>
> On 08-11-2016 18:29, Stephan Mueller wrote:
>> Am Dienstag, 8. November 2016, 17:16:38 CET schrieb Harsh Jain:
>>
>> Hi Harsh,
>>
>>> On 08-11-2016 16:45, Stephan Mueller wrote:
 Am Donnerstag, 27. Oktober 2016, 15:36:08 CET schrieb Harsh Jain:

 Hi Harsh,

>>> +static void chcr_verify_tag(struct aead_request *req, u8 *input, int
>>> *err)
>>> +{
>>> +   u8 temp[SHA512_DIGEST_SIZE];
>>> +   struct crypto_aead *tfm = crypto_aead_reqtfm(req);
>>> +   int authsize = crypto_aead_authsize(tfm);
>>> +   struct cpl_fw6_pld *fw6_pld;
>>> +   int cmp = 0;
>>> +
>>> +   fw6_pld = (struct cpl_fw6_pld *)input;
>>> +   if ((get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106) 
>>> ||
>>> +   (get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_GCM)) {
>>> +   cmp = memcmp(_pld->data[2], (fw6_pld + 1), 
>>> authsize);
>>> +   } else {
>>> +
>>> +   sg_pcopy_to_buffer(req->src, sg_nents(req->src), temp,
>>> +   authsize, req->assoclen +
>>> +   req->cryptlen - authsize);
>> I am wondering whether the math is correct here in any case. It is
>> permissible that we have an AAD size of 0 and even a zero-sized
>> ciphertext. How is such scenario covered here?
> Here we are trying to copy user supplied tag to local buffer(temp) for
> decrypt operation only. relative index of tag in src sg list will not
> change when AAD is zero and in decrypt operation cryptlen > authsize.
 I am just wondering where this is checked. Since all of these
 implementations are directly accessible from unprivileged user space, we
 should be careful.
>>> chcr_verify_tag() will be called when req->verify is set to "VERIFY_SW", 
>>> same will set in decrypt callback function of Algo(like chcr_aead_decrypt)
>>> only. It will ensure calling of chcr_verify_tag() in de-crypt operation
>>> only.
>> I think that limiting to the decryption path may not be enough. What happens 
>> if a caller sets some assoclen, but when invoking the decryption operation 
>> it 
>> provides input data that is smaller than the assoclen? The API allows this 
>> scenario.
> If I understand correctly, in this case passed sg list will be smaller. We 
> should return with error -EINVAL at entry point only (like create_gcm_wr), 
> control should not reach to chcr_verify_tag().
I had a look in software implementation for check related to aad len > src sg 
list.  I doubt same is not handled in software also. See  below
In "crypto_authenc_encrypt" if assoclen passed to 
"scatterwalk_ffwd" is greater than src. It may panic with NULL pointer 
exception.

 I will add  this check in V2 of chcr driver.

>
>> Ciao
>> Stephan

--
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

Re: [PATCH 6/6] Add support for AEAD algos.

[Harsh Jain]

On 08-11-2016 18:29, Stephan Mueller wrote:
> Am Dienstag, 8. November 2016, 17:16:38 CET schrieb Harsh Jain:
>
> Hi Harsh,
>
>> On 08-11-2016 16:45, Stephan Mueller wrote:
>>> Am Donnerstag, 27. Oktober 2016, 15:36:08 CET schrieb Harsh Jain:
>>>
>>> Hi Harsh,
>>>
>> +static void chcr_verify_tag(struct aead_request *req, u8 *input, int
>> *err)
>> +{
>> +u8 temp[SHA512_DIGEST_SIZE];
>> +struct crypto_aead *tfm = crypto_aead_reqtfm(req);
>> +int authsize = crypto_aead_authsize(tfm);
>> +struct cpl_fw6_pld *fw6_pld;
>> +int cmp = 0;
>> +
>> +fw6_pld = (struct cpl_fw6_pld *)input;
>> +if ((get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106) 
>> ||
>> +(get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_GCM)) {
>> +cmp = memcmp(_pld->data[2], (fw6_pld + 1), 
>> authsize);
>> +} else {
>> +
>> +sg_pcopy_to_buffer(req->src, sg_nents(req->src), temp,
>> +authsize, req->assoclen +
>> +req->cryptlen - authsize);
> I am wondering whether the math is correct here in any case. It is
> permissible that we have an AAD size of 0 and even a zero-sized
> ciphertext. How is such scenario covered here?
 Here we are trying to copy user supplied tag to local buffer(temp) for
 decrypt operation only. relative index of tag in src sg list will not
 change when AAD is zero and in decrypt operation cryptlen > authsize.
>>> I am just wondering where this is checked. Since all of these
>>> implementations are directly accessible from unprivileged user space, we
>>> should be careful.
>> chcr_verify_tag() will be called when req->verify is set to "VERIFY_SW", 
>> same will set in decrypt callback function of Algo(like chcr_aead_decrypt)
>> only. It will ensure calling of chcr_verify_tag() in de-crypt operation
>> only.
> I think that limiting to the decryption path may not be enough. What happens 
> if a caller sets some assoclen, but when invoking the decryption operation it 
> provides input data that is smaller than the assoclen? The API allows this 
> scenario.
If I understand correctly, in this case passed sg list will be smaller. We 
should return with error -EINVAL at entry point only (like create_gcm_wr), 
control should not reach to chcr_verify_tag().

>
> Ciao
> Stephan

--
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

Re: [PATCH 6/6] Add support for AEAD algos.

[Stephan Mueller]

Am Dienstag, 8. November 2016, 17:16:38 CET schrieb Harsh Jain:

Hi Harsh,

> On 08-11-2016 16:45, Stephan Mueller wrote:
> > Am Donnerstag, 27. Oktober 2016, 15:36:08 CET schrieb Harsh Jain:
> > 
> > Hi Harsh,
> > 
>  +static void chcr_verify_tag(struct aead_request *req, u8 *input, int
>  *err)
>  +{
>  +u8 temp[SHA512_DIGEST_SIZE];
>  +struct crypto_aead *tfm = crypto_aead_reqtfm(req);
>  +int authsize = crypto_aead_authsize(tfm);
>  +struct cpl_fw6_pld *fw6_pld;
>  +int cmp = 0;
>  +
>  +fw6_pld = (struct cpl_fw6_pld *)input;
>  +if ((get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106) 
>  ||
>  +(get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_GCM)) {
>  +cmp = memcmp(_pld->data[2], (fw6_pld + 1), 
>  authsize);
>  +} else {
>  +
>  +sg_pcopy_to_buffer(req->src, sg_nents(req->src), temp,
>  +authsize, req->assoclen +
>  +req->cryptlen - authsize);
> >>> 
> >>> I am wondering whether the math is correct here in any case. It is
> >>> permissible that we have an AAD size of 0 and even a zero-sized
> >>> ciphertext. How is such scenario covered here?
> >> 
> >> Here we are trying to copy user supplied tag to local buffer(temp) for
> >> decrypt operation only. relative index of tag in src sg list will not
> >> change when AAD is zero and in decrypt operation cryptlen > authsize.
> > 
> > I am just wondering where this is checked. Since all of these
> > implementations are directly accessible from unprivileged user space, we
> > should be careful.
> chcr_verify_tag() will be called when req->verify is set to "VERIFY_SW", 
> same will set in decrypt callback function of Algo(like chcr_aead_decrypt)
> only. It will ensure calling of chcr_verify_tag() in de-crypt operation
> only.

I think that limiting to the decryption path may not be enough. What happens 
if a caller sets some assoclen, but when invoking the decryption operation it 
provides input data that is smaller than the assoclen? The API allows this 
scenario.

Ciao
Stephan
--
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

Re: [PATCH 6/6] Add support for AEAD algos.

[Harsh Jain]

On 08-11-2016 16:45, Stephan Mueller wrote:
> Am Donnerstag, 27. Oktober 2016, 15:36:08 CET schrieb Harsh Jain:
>
> Hi Harsh,
>
 +static void chcr_verify_tag(struct aead_request *req, u8 *input, int
 *err)
 +{
 +  u8 temp[SHA512_DIGEST_SIZE];
 +  struct crypto_aead *tfm = crypto_aead_reqtfm(req);
 +  int authsize = crypto_aead_authsize(tfm);
 +  struct cpl_fw6_pld *fw6_pld;
 +  int cmp = 0;
 +
 +  fw6_pld = (struct cpl_fw6_pld *)input;
 +  if ((get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106) ||
 +  (get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_GCM)) {
 +  cmp = memcmp(_pld->data[2], (fw6_pld + 1), authsize);
 +  } else {
 +
 +  sg_pcopy_to_buffer(req->src, sg_nents(req->src), temp,
 +  authsize, req->assoclen +
 +  req->cryptlen - authsize);
>>> I am wondering whether the math is correct here in any case. It is
>>> permissible that we have an AAD size of 0 and even a zero-sized
>>> ciphertext. How is such scenario covered here?
>> Here we are trying to copy user supplied tag to local buffer(temp) for
>> decrypt operation only. relative index of tag in src sg list will not
>> change when AAD is zero and in decrypt operation cryptlen > authsize.
> I am just wondering where this is checked. Since all of these implementations 
> are directly accessible from unprivileged user space, we should be careful.
chcr_verify_tag() will be called when req->verify is set to "VERIFY_SW",  same 
will set in decrypt callback function of Algo(like chcr_aead_decrypt) only. It 
will ensure calling of chcr_verify_tag() in de-crypt operation only.


>
 +  cmp = memcmp(temp, (fw6_pld + 1), authsize);
>>> I would guess in both cases memcmp should be replaced with crypto_memneq
>> Yes can be done
>>
 +  }
 +  if (cmp)
 +  *err = -EBADMSG;
 +  else
 +  *err = 0;
>>> What do you think about memzero_explicit(tmp)?
>> No Idea why we needs explicitly setting of zero for local variable.  Please
>> share some online resources to understand this.
> In dumps, the stack is also produced. Yet I see that stack memory is very 
> volatile and thus will be overwritten soon. Thus my common approach for 
> sensitive data is that heap variables must be zeroized. Stack variables are 
> suggested to be zeroized. As far as I understand the code, temp will hold a 
> copy of the tag value, i.e. a public piece of information. If this is 
> correct, 
> that I concur that a memset may not be needed after all.
Yes, temp contains user supplied tag. We can ignore memset here. I will review 
the other function weather they need similar memset or not.
>
> Ciao
> Stephan

--
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

Re: [PATCH 6/6] Add support for AEAD algos.

[Stephan Mueller]

Am Donnerstag, 27. Oktober 2016, 15:36:08 CET schrieb Harsh Jain:

Hi Harsh,

> >> +static void chcr_verify_tag(struct aead_request *req, u8 *input, int
> >> *err)
> >> +{
> >> +  u8 temp[SHA512_DIGEST_SIZE];
> >> +  struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> >> +  int authsize = crypto_aead_authsize(tfm);
> >> +  struct cpl_fw6_pld *fw6_pld;
> >> +  int cmp = 0;
> >> +
> >> +  fw6_pld = (struct cpl_fw6_pld *)input;
> >> +  if ((get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106) ||
> >> +  (get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_GCM)) {
> >> +  cmp = memcmp(_pld->data[2], (fw6_pld + 1), authsize);
> >> +  } else {
> >> +
> >> +  sg_pcopy_to_buffer(req->src, sg_nents(req->src), temp,
> >> +  authsize, req->assoclen +
> >> +  req->cryptlen - authsize);
> > 
> > I am wondering whether the math is correct here in any case. It is
> > permissible that we have an AAD size of 0 and even a zero-sized
> > ciphertext. How is such scenario covered here?
> 
> Here we are trying to copy user supplied tag to local buffer(temp) for
> decrypt operation only. relative index of tag in src sg list will not
> change when AAD is zero and in decrypt operation cryptlen > authsize.

I am just wondering where this is checked. Since all of these implementations 
are directly accessible from unprivileged user space, we should be careful.

> >> +  cmp = memcmp(temp, (fw6_pld + 1), authsize);
> > 
> > I would guess in both cases memcmp should be replaced with crypto_memneq
> 
> Yes can be done
> 
> >> +  }
> >> +  if (cmp)
> >> +  *err = -EBADMSG;
> >> +  else
> >> +  *err = 0;
> > 
> > What do you think about memzero_explicit(tmp)?
> 
> No Idea why we needs explicitly setting of zero for local variable.  Please
> share some online resources to understand this.

In dumps, the stack is also produced. Yet I see that stack memory is very 
volatile and thus will be overwritten soon. Thus my common approach for 
sensitive data is that heap variables must be zeroized. Stack variables are 
suggested to be zeroized. As far as I understand the code, temp will hold a 
copy of the tag value, i.e. a public piece of information. If this is correct, 
that I concur that a memset may not be needed after all.

Ciao
Stephan
--
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

Re: [PATCH 6/6] Add support for AEAD algos.

[Harsh Jain]

On 14-10-2016 19:54, Stephan Mueller wrote:
> Am Donnerstag, 13. Oktober 2016, 16:39:39 CEST schrieb Harsh Jain:
>
> Hi Harsh,
>
>> Add support for following AEAD algos.
>>  GCM,CCM,RFC4106,RFC4309,authenc(hmac(shaXXX),cbc(aes)).
>>
>> Signed-off-by: Harsh Jain 
>> ---
>>  drivers/crypto/chelsio/Kconfig   |1 +
>>  drivers/crypto/chelsio/chcr_algo.c   | 1466
>> +- drivers/crypto/chelsio/chcr_algo.h   |  
>> 16 +-
>>  drivers/crypto/chelsio/chcr_core.c   |8 +-
>>  drivers/crypto/chelsio/chcr_core.h   |2 -
>>  drivers/crypto/chelsio/chcr_crypto.h |   90 ++-
>>  6 files changed, 1541 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/crypto/chelsio/Kconfig b/drivers/crypto/chelsio/Kconfig
>> index 4ce67fb..3e104f5 100644
>> --- a/drivers/crypto/chelsio/Kconfig
>> +++ b/drivers/crypto/chelsio/Kconfig
>> @@ -4,6 +4,7 @@ config CRYPTO_DEV_CHELSIO
>>  select CRYPTO_SHA1
>>  select CRYPTO_SHA256
>>  select CRYPTO_SHA512
>> +select CRYPTO_AUTHENC
>>  ---help---
>>The Chelsio Crypto Co-processor driver for T6 adapters.
>>
>> diff --git a/drivers/crypto/chelsio/chcr_algo.c
>> b/drivers/crypto/chelsio/chcr_algo.c index 18385d6..cffc38f 100644
>> --- a/drivers/crypto/chelsio/chcr_algo.c
>> +++ b/drivers/crypto/chelsio/chcr_algo.c
>> @@ -54,6 +54,12 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>>  #include 
>>
>>  #include "t4fw_api.h"
>> @@ -62,6 +68,11 @@
>>  #include "chcr_algo.h"
>>  #include "chcr_crypto.h"
>>
>> +static inline  struct chcr_aead_ctx *AEAD_CTX(struct chcr_context *ctx)
>> +{
>> +return ctx->crypto_ctx->aeadctx;
>> +}
>> +
>>  static inline struct ablk_ctx *ABLK_CTX(struct chcr_context *ctx)
>>  {
>>  return ctx->crypto_ctx->ablkctx;
>> @@ -72,6 +83,16 @@ static inline struct hmac_ctx *HMAC_CTX(struct
>> chcr_context *ctx) return ctx->crypto_ctx->hmacctx;
>>  }
>>
>> +static inline struct chcr_gcm_ctx *GCM_CTX(struct chcr_aead_ctx *gctx)
>> +{
>> +return gctx->ctx->gcm;
>> +}
>> +
>> +static inline struct chcr_authenc_ctx *AUTHENC_CTX(struct chcr_aead_ctx
>> *gctx) +{
>> +return gctx->ctx->authenc;
>> +}
>> +
>>  static inline struct uld_ctx *ULD_CTX(struct chcr_context *ctx)
>>  {
>>  return ctx->dev->u_ctx;
>> @@ -94,12 +115,37 @@ static inline unsigned int sgl_len(unsigned int n)
>>  return (3 * n) / 2 + (n & 1) + 2;
>>  }
>>
>> +static void chcr_verify_tag(struct aead_request *req, u8 *input, int *err)
>> +{
>> +u8 temp[SHA512_DIGEST_SIZE];
>> +struct crypto_aead *tfm = crypto_aead_reqtfm(req);
>> +int authsize = crypto_aead_authsize(tfm);
>> +struct cpl_fw6_pld *fw6_pld;
>> +int cmp = 0;
>> +
>> +fw6_pld = (struct cpl_fw6_pld *)input;
>> +if ((get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106) ||
>> +(get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_GCM)) {
>> +cmp = memcmp(_pld->data[2], (fw6_pld + 1), authsize);
>> +} else {
>> +
>> +sg_pcopy_to_buffer(req->src, sg_nents(req->src), temp,
>> +authsize, req->assoclen +
>> +req->cryptlen - authsize);
> I am wondering whether the math is correct here in any case. It is 
> permissible 
> that we have an AAD size of 0 and even a zero-sized ciphertext. How is such 
> scenario covered here?
Here we are trying to copy user supplied tag to local buffer(temp) for decrypt 
operation only. relative index of tag in src sg list
will not change when AAD is zero and in decrypt operation cryptlen > authsize.
>
>> +cmp = memcmp(temp, (fw6_pld + 1), authsize);
> I would guess in both cases memcmp should be replaced with crypto_memneq
Yes can be done

>
>> +}
>> +if (cmp)
>> +*err = -EBADMSG;
>> +else
>> +*err = 0;
> What do you think about memzero_explicit(tmp)?
No Idea why we needs explicitly setting of zero for local variable.  Please 
share some online resources to understand this.

>
>> +}
>> +
>>  /*
>>   *  chcr_handle_resp - Unmap the DMA buffers associated with the request
>>   *  @req: crypto request
>>   */
>>  int chcr_handle_resp(struct crypto_async_request *req, unsigned char
>> *input, - int error_status)
>> + int err)
>>  {
>>  struct crypto_tfm *tfm = req->tfm;
>>  struct chcr_context *ctx = crypto_tfm_ctx(tfm);
>> @@ -109,11 +155,27 @@ int chcr_handle_resp(struct crypto_async_request *req,
>> unsigned char *input, unsigned int digestsize, updated_digestsize;
>>
>>  switch (tfm->__crt_alg->cra_flags & CRYPTO_ALG_TYPE_MASK) {
>> +case CRYPTO_ALG_TYPE_AEAD:
>> +ctx_req.req.aead_req = (struct aead_request *)req;
>> +ctx_req.ctx.reqctx = aead_request_ctx(ctx_req.req.aead_req);
>> +dma_unmap_sg(_ctx->lldi.pdev->dev, ctx_req.req.aead_req->dst,
>> + 

Re: [PATCH 6/6] Add support for AEAD algos.

[Stephan Mueller]

Am Donnerstag, 13. Oktober 2016, 16:39:39 CEST schrieb Harsh Jain:

Hi Harsh,

> Add support for following AEAD algos.
>  GCM,CCM,RFC4106,RFC4309,authenc(hmac(shaXXX),cbc(aes)).
> 
> Signed-off-by: Harsh Jain 
> ---
>  drivers/crypto/chelsio/Kconfig   |1 +
>  drivers/crypto/chelsio/chcr_algo.c   | 1466
> +- drivers/crypto/chelsio/chcr_algo.h   |  
> 16 +-
>  drivers/crypto/chelsio/chcr_core.c   |8 +-
>  drivers/crypto/chelsio/chcr_core.h   |2 -
>  drivers/crypto/chelsio/chcr_crypto.h |   90 ++-
>  6 files changed, 1541 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/crypto/chelsio/Kconfig b/drivers/crypto/chelsio/Kconfig
> index 4ce67fb..3e104f5 100644
> --- a/drivers/crypto/chelsio/Kconfig
> +++ b/drivers/crypto/chelsio/Kconfig
> @@ -4,6 +4,7 @@ config CRYPTO_DEV_CHELSIO
>   select CRYPTO_SHA1
>   select CRYPTO_SHA256
>   select CRYPTO_SHA512
> + select CRYPTO_AUTHENC
>   ---help---
> The Chelsio Crypto Co-processor driver for T6 adapters.
> 
> diff --git a/drivers/crypto/chelsio/chcr_algo.c
> b/drivers/crypto/chelsio/chcr_algo.c index 18385d6..cffc38f 100644
> --- a/drivers/crypto/chelsio/chcr_algo.c
> +++ b/drivers/crypto/chelsio/chcr_algo.c
> @@ -54,6 +54,12 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
>  #include 
> 
>  #include "t4fw_api.h"
> @@ -62,6 +68,11 @@
>  #include "chcr_algo.h"
>  #include "chcr_crypto.h"
> 
> +static inline  struct chcr_aead_ctx *AEAD_CTX(struct chcr_context *ctx)
> +{
> + return ctx->crypto_ctx->aeadctx;
> +}
> +
>  static inline struct ablk_ctx *ABLK_CTX(struct chcr_context *ctx)
>  {
>   return ctx->crypto_ctx->ablkctx;
> @@ -72,6 +83,16 @@ static inline struct hmac_ctx *HMAC_CTX(struct
> chcr_context *ctx) return ctx->crypto_ctx->hmacctx;
>  }
> 
> +static inline struct chcr_gcm_ctx *GCM_CTX(struct chcr_aead_ctx *gctx)
> +{
> + return gctx->ctx->gcm;
> +}
> +
> +static inline struct chcr_authenc_ctx *AUTHENC_CTX(struct chcr_aead_ctx
> *gctx) +{
> + return gctx->ctx->authenc;
> +}
> +
>  static inline struct uld_ctx *ULD_CTX(struct chcr_context *ctx)
>  {
>   return ctx->dev->u_ctx;
> @@ -94,12 +115,37 @@ static inline unsigned int sgl_len(unsigned int n)
>   return (3 * n) / 2 + (n & 1) + 2;
>  }
> 
> +static void chcr_verify_tag(struct aead_request *req, u8 *input, int *err)
> +{
> + u8 temp[SHA512_DIGEST_SIZE];
> + struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> + int authsize = crypto_aead_authsize(tfm);
> + struct cpl_fw6_pld *fw6_pld;
> + int cmp = 0;
> +
> + fw6_pld = (struct cpl_fw6_pld *)input;
> + if ((get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106) ||
> + (get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_GCM)) {
> + cmp = memcmp(_pld->data[2], (fw6_pld + 1), authsize);
> + } else {
> +
> + sg_pcopy_to_buffer(req->src, sg_nents(req->src), temp,
> + authsize, req->assoclen +
> + req->cryptlen - authsize);

I am wondering whether the math is correct here in any case. It is permissible 
that we have an AAD size of 0 and even a zero-sized ciphertext. How is such 
scenario covered here?

> + cmp = memcmp(temp, (fw6_pld + 1), authsize);

I would guess in both cases memcmp should be replaced with crypto_memneq

> + }
> + if (cmp)
> + *err = -EBADMSG;
> + else
> + *err = 0;

What do you think about memzero_explicit(tmp)?

> +}
> +
>  /*
>   *   chcr_handle_resp - Unmap the DMA buffers associated with the request
>   *   @req: crypto request
>   */
>  int chcr_handle_resp(struct crypto_async_request *req, unsigned char
> *input, -  int error_status)
> +  int err)
>  {
>   struct crypto_tfm *tfm = req->tfm;
>   struct chcr_context *ctx = crypto_tfm_ctx(tfm);
> @@ -109,11 +155,27 @@ int chcr_handle_resp(struct crypto_async_request *req,
> unsigned char *input, unsigned int digestsize, updated_digestsize;
> 
>   switch (tfm->__crt_alg->cra_flags & CRYPTO_ALG_TYPE_MASK) {
> + case CRYPTO_ALG_TYPE_AEAD:
> + ctx_req.req.aead_req = (struct aead_request *)req;
> + ctx_req.ctx.reqctx = aead_request_ctx(ctx_req.req.aead_req);
> + dma_unmap_sg(_ctx->lldi.pdev->dev, ctx_req.req.aead_req->dst,
> +  ctx_req.ctx.reqctx->dst_nents, DMA_FROM_DEVICE);
> + if (ctx_req.ctx.reqctx->skb) {
> + kfree_skb(ctx_req.ctx.reqctx->skb);
> + ctx_req.ctx.reqctx->skb = NULL;
> + }
> + if (ctx_req.ctx.reqctx->verify == VERIFY_SW) {
> + chcr_verify_tag(ctx_req.req.aead_req, input,
> + );
> + ctx_req.ctx.reqctx->verify = VERIFY_HW;
> + }
> +