diff mbox series

crypto/mvsam: fix missed code change for mvsam

Message ID 20190705132349.7245-1-roy.fan.zhang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: akhil goyal
Headers
Series crypto/mvsam: fix missed code change for mvsam |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Fan Zhang July 5, 2019, 1:23 p.m. UTC 
  This patch fixes the missed "uint8_t *" to "const uint8_t *" xform
key data type change for mvsam driver.

Fixes: f3390532cf6a ("cryptodev: make xform key pointer constant")

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 drivers/crypto/mvsam/rte_mrvl_pmd.c     | 44 ++++++++++++++++++++++++++++++---
 drivers/crypto/mvsam/rte_mrvl_pmd_ops.c |  9 +++++++
 2 files changed, 49 insertions(+), 4 deletions(-)

Comments

Akhil Goyal July 5, 2019, 1:28 p.m. UTC | #1 
Hi Fan,

> 
> This patch fixes the missed "uint8_t *" to "const uint8_t *" xform
> key data type change for mvsam driver.
> 
> Fixes: f3390532cf6a ("cryptodev: make xform key pointer constant")
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>  drivers/crypto/mvsam/rte_mrvl_pmd.c     | 44
> ++++++++++++++++++++++++++++++---
>  drivers/crypto/mvsam/rte_mrvl_pmd_ops.c |  9 +++++++
>  2 files changed, 49 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/crypto/mvsam/rte_mrvl_pmd.c
> b/drivers/crypto/mvsam/rte_mrvl_pmd.c
> index c2ae82a26..65e4769f2 100644
> --- a/drivers/crypto/mvsam/rte_mrvl_pmd.c
> +++ b/drivers/crypto/mvsam/rte_mrvl_pmd.c
> @@ -222,6 +222,8 @@ static int
>  mrvl_crypto_set_cipher_session_parameters(struct mrvl_crypto_session *sess,
>  		const struct rte_crypto_sym_xform *cipher_xform)
>  {
> +	uint8_t *cipher_key;
> +
>  	/* Make sure we've got proper struct */
>  	if (cipher_xform->type != RTE_CRYPTO_SYM_XFORM_CIPHER) {
>  		MRVL_LOG(ERR, "Wrong xform struct provided!");
> @@ -256,8 +258,17 @@ mrvl_crypto_set_cipher_session_parameters(struct
> mrvl_crypto_session *sess,
>  		return -EINVAL;
>  	}
> 
> +	cipher_key = malloc(cipher_xform->cipher.key.length);
> +	if (cipher_key == NULL) {
> +		MRVL_LOG(ERR, "Insufficient memory!");
> +		return -ENOMEM;
> +	}
> +
> +	memcpy(cipher_key, cipher_xform->cipher.key,
> +			cipher_xform->cipher.key.length);
> +
>  	sess->sam_sess_params.cipher_key_len = cipher_xform-
> >cipher.key.length;
> -	sess->sam_sess_params.cipher_key = cipher_xform->cipher.key.data;
> +	sess->sam_sess_params.cipher_key = cipher_key;
> 
>  	return 0;
>  }
> @@ -273,6 +284,8 @@ static int
>  mrvl_crypto_set_auth_session_parameters(struct mrvl_crypto_session *sess,
>  		const struct rte_crypto_sym_xform *auth_xform)
>  {
> +	uint8_t *auth_key = NULL;
> +
>  	/* Make sure we've got proper struct */
>  	if (auth_xform->type != RTE_CRYPTO_SYM_XFORM_AUTH) {
>  		MRVL_LOG(ERR, "Wrong xform struct provided!");
> @@ -293,9 +306,21 @@ mrvl_crypto_set_auth_session_parameters(struct
> mrvl_crypto_session *sess,
>  		auth_map[auth_xform->auth.algo].auth_alg;
>  	sess->sam_sess_params.u.basic.auth_icv_len =
>  		auth_xform->auth.digest_length;
> +
> +	if (auth_xform->auth.key.length > 0) {
> +		auth_key = malloc(auth_xform->auth.key.length);
> +		if (auth_key == NULL) {
> +			MRVL_LOG(ERR, "Not enough memory!");
> +			return -EINVAL;
> +		}
> +
> +		memcpy(auth_key, auth_xform->auth.key.data,
> +				auth_xform->auth.key.length);
> +	}
> +
> +
Extra blank spaces.


>  	/* auth_key must be NULL if auth algorithm does not use HMAC */
> -	sess->sam_sess_params.auth_key = auth_xform->auth.key.length ?
> -					 auth_xform->auth.key.data : NULL;
> +	sess->sam_sess_params.auth_key = auth_key;
>  	sess->sam_sess_params.auth_key_len = auth_xform->auth.key.length;
> 
>  	return 0;
> @@ -312,6 +337,8 @@ static int
>  mrvl_crypto_set_aead_session_parameters(struct mrvl_crypto_session *sess,
>  		const struct rte_crypto_sym_xform *aead_xform)
>  {
> +	uint8_t *aead_key;
> +
>  	/* Make sure we've got proper struct */
>  	if (aead_xform->type != RTE_CRYPTO_SYM_XFORM_AEAD) {
>  		MRVL_LOG(ERR, "Wrong xform struct provided!");
> @@ -344,7 +371,16 @@ mrvl_crypto_set_aead_session_parameters(struct
> mrvl_crypto_session *sess,
>  		return -EINVAL;
>  	}
> 
> -	sess->sam_sess_params.cipher_key = aead_xform->aead.key.data;
> +	aead_key = malloc(aead_xform->aead.key.length);
> +	if (aead_key == NULL) {
> +		MRVL_LOG(ERR, "Insufficient memory!");
> +		return -ENOMEM;
> +	}
> +
> +	memcpy(aead_key, aead_xform->aead.key.data,
> +			aead_xform->aead.key.length);
> +
> +	sess->sam_sess_params.cipher_key = aead_key;
>  	sess->sam_sess_params.cipher_key_len = aead_xform-
> >aead.key.length;
> 
>  	if (sess->sam_sess_params.cipher_mode == SAM_CIPHER_GCM)
> diff --git a/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> b/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> index f6bf2cd4c..b334c7694 100644
> --- a/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> +++ b/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> @@ -751,6 +751,8 @@
> mrvl_crypto_pmd_sym_session_configure(__rte_unused struct rte_cryptodev
> *dev,
>  		return -ENOMEM;
>  	}
> 
> +	memset(sess_private_data, 0, sizeof(struct mrvl_crypto_session));
> +
>  	ret = mrvl_crypto_set_session_parameters(sess_private_data, xform);
>  	if (ret != 0) {
>  		MRVL_LOG(ERR, "Failed to configure session parameters!");
> @@ -769,6 +771,13 @@
> mrvl_crypto_pmd_sym_session_configure(__rte_unused struct rte_cryptodev
> *dev,
>  		return -EIO;
>  	}
> 
> +	/* free the keys memory allocated for session creation */
> +	if (mrvl_sess->sam_sess_params.cipher_key != NULL)
> +		free(mrvl_sess->sam_sess_params.cipher_key);
> +	if (mrvl_sess->sam_sess_params.auth_key != NULL)
> +		free(mrvl_sess->sam_sess_params.auth_key);
> +
> +
Extra blank and missing the aead key free.


>  	return 0;
>  }
> 
> --
> 2.14.5
Akhil Goyal July 5, 2019, 1:31 p.m. UTC | #2 
Hi Liron,

Following patch need to be merged today as it is fixing a build issue for a patch already merged.
Could you please review this patch? Sorry for a very short notice.

Thanks,
Akhil

> -----Original Message-----
> From: Fan Zhang <roy.fan.zhang@intel.com>
> Sent: Friday, July 5, 2019 6:54 PM
> To: dev@dpdk.org
> Cc: Akhil Goyal <akhil.goyal@nxp.com>; Fan Zhang <roy.fan.zhang@intel.com>
> Subject: [PATCH] crypto/mvsam: fix missed code change for mvsam
> 
> This patch fixes the missed "uint8_t *" to "const uint8_t *" xform
> key data type change for mvsam driver.
> 
> Fixes: f3390532cf6a ("cryptodev: make xform key pointer constant")
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>  drivers/crypto/mvsam/rte_mrvl_pmd.c     | 44
> ++++++++++++++++++++++++++++++---
>  drivers/crypto/mvsam/rte_mrvl_pmd_ops.c |  9 +++++++
>  2 files changed, 49 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/crypto/mvsam/rte_mrvl_pmd.c
> b/drivers/crypto/mvsam/rte_mrvl_pmd.c
> index c2ae82a26..65e4769f2 100644
> --- a/drivers/crypto/mvsam/rte_mrvl_pmd.c
> +++ b/drivers/crypto/mvsam/rte_mrvl_pmd.c
> @@ -222,6 +222,8 @@ static int
>  mrvl_crypto_set_cipher_session_parameters(struct mrvl_crypto_session *sess,
>  		const struct rte_crypto_sym_xform *cipher_xform)
>  {
> +	uint8_t *cipher_key;
> +
>  	/* Make sure we've got proper struct */
>  	if (cipher_xform->type != RTE_CRYPTO_SYM_XFORM_CIPHER) {
>  		MRVL_LOG(ERR, "Wrong xform struct provided!");
> @@ -256,8 +258,17 @@ mrvl_crypto_set_cipher_session_parameters(struct
> mrvl_crypto_session *sess,
>  		return -EINVAL;
>  	}
> 
> +	cipher_key = malloc(cipher_xform->cipher.key.length);
> +	if (cipher_key == NULL) {
> +		MRVL_LOG(ERR, "Insufficient memory!");
> +		return -ENOMEM;
> +	}
> +
> +	memcpy(cipher_key, cipher_xform->cipher.key,
> +			cipher_xform->cipher.key.length);
> +
>  	sess->sam_sess_params.cipher_key_len = cipher_xform-
> >cipher.key.length;
> -	sess->sam_sess_params.cipher_key = cipher_xform->cipher.key.data;
> +	sess->sam_sess_params.cipher_key = cipher_key;
> 
>  	return 0;
>  }
> @@ -273,6 +284,8 @@ static int
>  mrvl_crypto_set_auth_session_parameters(struct mrvl_crypto_session *sess,
>  		const struct rte_crypto_sym_xform *auth_xform)
>  {
> +	uint8_t *auth_key = NULL;
> +
>  	/* Make sure we've got proper struct */
>  	if (auth_xform->type != RTE_CRYPTO_SYM_XFORM_AUTH) {
>  		MRVL_LOG(ERR, "Wrong xform struct provided!");
> @@ -293,9 +306,21 @@ mrvl_crypto_set_auth_session_parameters(struct
> mrvl_crypto_session *sess,
>  		auth_map[auth_xform->auth.algo].auth_alg;
>  	sess->sam_sess_params.u.basic.auth_icv_len =
>  		auth_xform->auth.digest_length;
> +
> +	if (auth_xform->auth.key.length > 0) {
> +		auth_key = malloc(auth_xform->auth.key.length);
> +		if (auth_key == NULL) {
> +			MRVL_LOG(ERR, "Not enough memory!");
> +			return -EINVAL;
> +		}
> +
> +		memcpy(auth_key, auth_xform->auth.key.data,
> +				auth_xform->auth.key.length);
> +	}
> +
> +
>  	/* auth_key must be NULL if auth algorithm does not use HMAC */
> -	sess->sam_sess_params.auth_key = auth_xform->auth.key.length ?
> -					 auth_xform->auth.key.data : NULL;
> +	sess->sam_sess_params.auth_key = auth_key;
>  	sess->sam_sess_params.auth_key_len = auth_xform->auth.key.length;
> 
>  	return 0;
> @@ -312,6 +337,8 @@ static int
>  mrvl_crypto_set_aead_session_parameters(struct mrvl_crypto_session *sess,
>  		const struct rte_crypto_sym_xform *aead_xform)
>  {
> +	uint8_t *aead_key;
> +
>  	/* Make sure we've got proper struct */
>  	if (aead_xform->type != RTE_CRYPTO_SYM_XFORM_AEAD) {
>  		MRVL_LOG(ERR, "Wrong xform struct provided!");
> @@ -344,7 +371,16 @@ mrvl_crypto_set_aead_session_parameters(struct
> mrvl_crypto_session *sess,
>  		return -EINVAL;
>  	}
> 
> -	sess->sam_sess_params.cipher_key = aead_xform->aead.key.data;
> +	aead_key = malloc(aead_xform->aead.key.length);
> +	if (aead_key == NULL) {
> +		MRVL_LOG(ERR, "Insufficient memory!");
> +		return -ENOMEM;
> +	}
> +
> +	memcpy(aead_key, aead_xform->aead.key.data,
> +			aead_xform->aead.key.length);
> +
> +	sess->sam_sess_params.cipher_key = aead_key;
>  	sess->sam_sess_params.cipher_key_len = aead_xform-
> >aead.key.length;
> 
>  	if (sess->sam_sess_params.cipher_mode == SAM_CIPHER_GCM)
> diff --git a/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> b/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> index f6bf2cd4c..b334c7694 100644
> --- a/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> +++ b/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> @@ -751,6 +751,8 @@
> mrvl_crypto_pmd_sym_session_configure(__rte_unused struct rte_cryptodev
> *dev,
>  		return -ENOMEM;
>  	}
> 
> +	memset(sess_private_data, 0, sizeof(struct mrvl_crypto_session));
> +
>  	ret = mrvl_crypto_set_session_parameters(sess_private_data, xform);
>  	if (ret != 0) {
>  		MRVL_LOG(ERR, "Failed to configure session parameters!");
> @@ -769,6 +771,13 @@
> mrvl_crypto_pmd_sym_session_configure(__rte_unused struct rte_cryptodev
> *dev,
>  		return -EIO;
>  	}
> 
> +	/* free the keys memory allocated for session creation */
> +	if (mrvl_sess->sam_sess_params.cipher_key != NULL)
> +		free(mrvl_sess->sam_sess_params.cipher_key);
> +	if (mrvl_sess->sam_sess_params.auth_key != NULL)
> +		free(mrvl_sess->sam_sess_params.auth_key);
> +
> +
>  	return 0;
>  }
> 
> --
> 2.14.5
Fan Zhang July 5, 2019, 1:57 p.m. UTC | #3 
Hi Akhil,

Thanks for the review, comments inline.

Regards,
Fan

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Friday, July 5, 2019 2:28 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH] crypto/mvsam: fix missed code change for mvsam
> 
> Hi Fan,
> 
> >
...
> Extra blank spaces.

Will change in v2. 

> 

...

> > -	sess->sam_sess_params.cipher_key = aead_xform->aead.key.data;
> > +	aead_key = malloc(aead_xform->aead.key.length);
> > +	if (aead_key == NULL) {
> > +		MRVL_LOG(ERR, "Insufficient memory!");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	memcpy(aead_key, aead_xform->aead.key.data,
> > +			aead_xform->aead.key.length);
> > +
> > +	sess->sam_sess_params.cipher_key = aead_key;
> >  	sess->sam_sess_params.cipher_key_len = aead_xform-
> > >aead.key.length;
> >
> >  	if (sess->sam_sess_params.cipher_mode == SAM_CIPHER_GCM)
> diff --git
> > a/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> > b/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> > index f6bf2cd4c..b334c7694 100644
> > --- a/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> > +++ b/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> > @@ -751,6 +751,8 @@


> >
> > +	/* free the keys memory allocated for session creation */
> > +	if (mrvl_sess->sam_sess_params.cipher_key != NULL)
> > +		free(mrvl_sess->sam_sess_params.cipher_key);
> > +	if (mrvl_sess->sam_sess_params.auth_key != NULL)
> > +		free(mrvl_sess->sam_sess_params.auth_key);
> > +
> > +
> Extra blank and missing the aead key free.

During the session config the driver used cipher_key field to store the aead key. So no matter it is cipher key or aead key the memory is freed here.
Will fix the extra blank line in v2.

> 
> 
> >  	return 0;
> >  }
> >
> > --
> > 2.14.5
Liron Himi July 7, 2019, 5:41 a.m. UTC | #4 
-----Original Message-----
From: Akhil Goyal <akhil.goyal@nxp.com> 
Sent: Friday, July 5, 2019 16:32
To: Fan Zhang <roy.fan.zhang@intel.com>; dev@dpdk.org; Liron Himi <lironh@marvell.com>
Subject: [EXT] RE: [PATCH] crypto/mvsam: fix missed code change for mvsam

External Email

----------------------------------------------------------------------
Hi Liron,

Following patch need to be merged today as it is fixing a build issue for a patch already merged.
Could you please review this patch? Sorry for a very short notice.

Thanks,
Akhil

> -----Original Message-----
> From: Fan Zhang <roy.fan.zhang@intel.com>
> Sent: Friday, July 5, 2019 6:54 PM
> To: dev@dpdk.org
> Cc: Akhil Goyal <akhil.goyal@nxp.com>; Fan Zhang 
> <roy.fan.zhang@intel.com>
> Subject: [PATCH] crypto/mvsam: fix missed code change for mvsam
> 
> This patch fixes the missed "uint8_t *" to "const uint8_t *" xform key 
> data type change for mvsam driver.
> 
> Fixes: f3390532cf6a ("cryptodev: make xform key pointer constant")
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>  drivers/crypto/mvsam/rte_mrvl_pmd.c     | 44
> ++++++++++++++++++++++++++++++---
>  drivers/crypto/mvsam/rte_mrvl_pmd_ops.c |  9 +++++++
>  2 files changed, 49 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/crypto/mvsam/rte_mrvl_pmd.c
> b/drivers/crypto/mvsam/rte_mrvl_pmd.c
> index c2ae82a26..65e4769f2 100644
> --- a/drivers/crypto/mvsam/rte_mrvl_pmd.c
> +++ b/drivers/crypto/mvsam/rte_mrvl_pmd.c
> @@ -222,6 +222,8 @@ static int
>  mrvl_crypto_set_cipher_session_parameters(struct mrvl_crypto_session *sess,
>  		const struct rte_crypto_sym_xform *cipher_xform)  {
> +	uint8_t *cipher_key;
> +
>  	/* Make sure we've got proper struct */
>  	if (cipher_xform->type != RTE_CRYPTO_SYM_XFORM_CIPHER) {
>  		MRVL_LOG(ERR, "Wrong xform struct provided!"); @@ -256,8 +258,17 @@ 
> mrvl_crypto_set_cipher_session_parameters(struct
> mrvl_crypto_session *sess,
>  		return -EINVAL;
>  	}
> 
> +	cipher_key = malloc(cipher_xform->cipher.key.length);
> +	if (cipher_key == NULL) {
> +		MRVL_LOG(ERR, "Insufficient memory!");
> +		return -ENOMEM;
> +	}
> +
> +	memcpy(cipher_key, cipher_xform->cipher.key,
> +			cipher_xform->cipher.key.length);
> +
>  	sess->sam_sess_params.cipher_key_len = cipher_xform-
> >cipher.key.length;
> -	sess->sam_sess_params.cipher_key = cipher_xform->cipher.key.data;
> +	sess->sam_sess_params.cipher_key = cipher_key;
> 
>  	return 0;
>  }
> @@ -273,6 +284,8 @@ static int
>  mrvl_crypto_set_auth_session_parameters(struct mrvl_crypto_session *sess,
>  		const struct rte_crypto_sym_xform *auth_xform)  {
> +	uint8_t *auth_key = NULL;
> +
>  	/* Make sure we've got proper struct */
>  	if (auth_xform->type != RTE_CRYPTO_SYM_XFORM_AUTH) {
>  		MRVL_LOG(ERR, "Wrong xform struct provided!"); @@ -293,9 +306,21 @@ 
> mrvl_crypto_set_auth_session_parameters(struct
> mrvl_crypto_session *sess,
>  		auth_map[auth_xform->auth.algo].auth_alg;
>  	sess->sam_sess_params.u.basic.auth_icv_len =
>  		auth_xform->auth.digest_length;
> +
> +	if (auth_xform->auth.key.length > 0) {
> +		auth_key = malloc(auth_xform->auth.key.length);
> +		if (auth_key == NULL) {
> +			MRVL_LOG(ERR, "Not enough memory!");
> +			return -EINVAL;
> +		}
> +
> +		memcpy(auth_key, auth_xform->auth.key.data,
> +				auth_xform->auth.key.length);
> +	}
> +
> +
>  	/* auth_key must be NULL if auth algorithm does not use HMAC */
> -	sess->sam_sess_params.auth_key = auth_xform->auth.key.length ?
> -					 auth_xform->auth.key.data : NULL;
> +	sess->sam_sess_params.auth_key = auth_key;
>  	sess->sam_sess_params.auth_key_len = auth_xform->auth.key.length;
> 
>  	return 0;
> @@ -312,6 +337,8 @@ static int
>  mrvl_crypto_set_aead_session_parameters(struct mrvl_crypto_session *sess,
>  		const struct rte_crypto_sym_xform *aead_xform)  {
> +	uint8_t *aead_key;
> +
>  	/* Make sure we've got proper struct */
>  	if (aead_xform->type != RTE_CRYPTO_SYM_XFORM_AEAD) {
>  		MRVL_LOG(ERR, "Wrong xform struct provided!"); @@ -344,7 +371,16 @@ 
> mrvl_crypto_set_aead_session_parameters(struct
> mrvl_crypto_session *sess,
>  		return -EINVAL;
>  	}
> 
> -	sess->sam_sess_params.cipher_key = aead_xform->aead.key.data;
> +	aead_key = malloc(aead_xform->aead.key.length);
> +	if (aead_key == NULL) {
> +		MRVL_LOG(ERR, "Insufficient memory!");
> +		return -ENOMEM;
> +	}
> +
> +	memcpy(aead_key, aead_xform->aead.key.data,
> +			aead_xform->aead.key.length);
> +
> +	sess->sam_sess_params.cipher_key = aead_key;
>  	sess->sam_sess_params.cipher_key_len = aead_xform-
> >aead.key.length;
> 
>  	if (sess->sam_sess_params.cipher_mode == SAM_CIPHER_GCM) diff --git 
> a/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> b/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> index f6bf2cd4c..b334c7694 100644
> --- a/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> +++ b/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
> @@ -751,6 +751,8 @@
> mrvl_crypto_pmd_sym_session_configure(__rte_unused struct 
> rte_cryptodev *dev,
>  		return -ENOMEM;
>  	}
> 
> +	memset(sess_private_data, 0, sizeof(struct mrvl_crypto_session));
> +
>  	ret = mrvl_crypto_set_session_parameters(sess_private_data, xform);
>  	if (ret != 0) {
>  		MRVL_LOG(ERR, "Failed to configure session parameters!"); @@ -769,6 
> +771,13 @@ mrvl_crypto_pmd_sym_session_configure(__rte_unused struct 
> rte_cryptodev *dev,
>  		return -EIO;
>  	}
> 
> +	/* free the keys memory allocated for session creation */
> +	if (mrvl_sess->sam_sess_params.cipher_key != NULL)
> +		free(mrvl_sess->sam_sess_params.cipher_key);
> +	if (mrvl_sess->sam_sess_params.auth_key != NULL)
> +		free(mrvl_sess->sam_sess_params.auth_key);
> +
> +
>  	return 0;
>  }
> 
> --
> 2.14.5

Based on Fan's comments

acked-by: Liron Himi <lironh@marvell.com>
Thomas Monjalon July 8, 2019, 10:26 a.m. UTC | #5 
07/07/2019 07:41, Liron Himi:
> From: Akhil Goyal <akhil.goyal@nxp.com> 
> > This patch fixes the missed "uint8_t *" to "const uint8_t *" xform key 
> > data type change for mvsam driver.
> > 
> > Fixes: f3390532cf6a ("cryptodev: make xform key pointer constant")
> > 
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > ---
> Based on Fan's comments
> 
> acked-by: Liron Himi <lironh@marvell.com>

So we are OK to merge this patch with
"cryptodev: make xform key pointer constant"
in next-crypto tree?
Akhil Goyal July 8, 2019, 10:31 a.m. UTC | #6 
> 
> 07/07/2019 07:41, Liron Himi:
> > From: Akhil Goyal <akhil.goyal@nxp.com>
> > > This patch fixes the missed "uint8_t *" to "const uint8_t *" xform key
> > > data type change for mvsam driver.
> > >
> > > Fixes: f3390532cf6a ("cryptodev: make xform key pointer constant")
> > >
> > > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > > ---
> > Based on Fan's comments
> >
> > acked-by: Liron Himi <lironh@marvell.com>
> 
> So we are OK to merge this patch with
> "cryptodev: make xform key pointer constant"
> in next-crypto tree?
> 
I have just squashed this with the original commit.
Thomas Monjalon July 8, 2019, 3:22 p.m. UTC | #7 
08/07/2019 12:26, Thomas Monjalon:
> 07/07/2019 07:41, Liron Himi:
> > From: Akhil Goyal <akhil.goyal@nxp.com> 
> > > This patch fixes the missed "uint8_t *" to "const uint8_t *" xform key 
> > > data type change for mvsam driver.
> > > 
> > > Fixes: f3390532cf6a ("cryptodev: make xform key pointer constant")
> > > 
> > > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > > ---
> > Based on Fan's comments
> > 
> > acked-by: Liron Himi <lironh@marvell.com>
> 
> So we are OK to merge this patch with
> "cryptodev: make xform key pointer constant"
> in next-crypto tree?

There is still a compilation error:

drivers/crypto/mvsam/rte_mrvl_pmd.c:267:41: error:
	incompatible type for argument 2 of 'memcpy'
  memcpy(cipher_key, cipher_xform->cipher.key,
                     ~~~~~~~~~~~~~~~~~~~~^~~~

Seen when compiling arm64-thunderx-linux-gcc
Akhil Goyal July 12, 2019, 6:31 a.m. UTC | #8 
Hi Fan,

> 
> 08/07/2019 12:26, Thomas Monjalon:
> > 07/07/2019 07:41, Liron Himi:
> > > From: Akhil Goyal <akhil.goyal@nxp.com>
> > > > This patch fixes the missed "uint8_t *" to "const uint8_t *" xform key
> > > > data type change for mvsam driver.
> > > >
> > > > Fixes: f3390532cf6a ("cryptodev: make xform key pointer constant")
> > > >
> > > > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > > > ---
> > > Based on Fan's comments
> > >
> > > acked-by: Liron Himi <lironh@marvell.com>
> >
> > So we are OK to merge this patch with
> > "cryptodev: make xform key pointer constant"
> > in next-crypto tree?
> 
> There is still a compilation error:
> 
> drivers/crypto/mvsam/rte_mrvl_pmd.c:267:41: error:
> 	incompatible type for argument 2 of 'memcpy'
>   memcpy(cipher_key, cipher_xform->cipher.key,
>                      ~~~~~~~~~~~~~~~~~~~~^~~~
> 
> Seen when compiling arm64-thunderx-linux-gcc
> 

Could you send a proper fix to this issue.

Thanks,
Akhil
Akhil Goyal July 15, 2019, 12:45 p.m. UTC | #9 
Hi Fan/Liron,

Could you please send the incremental patch ASAP. Otherwise I need to remove this patch from the subtree.
I don't have environment to test this.

Regards,
Akhil


> 
> Hi Fan,
> 
> >
> > 08/07/2019 12:26, Thomas Monjalon:
> > > 07/07/2019 07:41, Liron Himi:
> > > > From: Akhil Goyal <akhil.goyal@nxp.com>
> > > > > This patch fixes the missed "uint8_t *" to "const uint8_t *" xform key
> > > > > data type change for mvsam driver.
> > > > >
> > > > > Fixes: f3390532cf6a ("cryptodev: make xform key pointer constant")
> > > > >
> > > > > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > > > > ---
> > > > Based on Fan's comments
> > > >
> > > > acked-by: Liron Himi <lironh@marvell.com>
> > >
> > > So we are OK to merge this patch with
> > > "cryptodev: make xform key pointer constant"
> > > in next-crypto tree?
> >
> > There is still a compilation error:
> >
> > drivers/crypto/mvsam/rte_mrvl_pmd.c:267:41: error:
> > 	incompatible type for argument 2 of 'memcpy'
> >   memcpy(cipher_key, cipher_xform->cipher.key,
> >                      ~~~~~~~~~~~~~~~~~~~~^~~~
> >
> > Seen when compiling arm64-thunderx-linux-gcc
> >
> 
> Could you send a proper fix to this issue.
> 
> Thanks,
> Akhil
diff mbox series

Patch

diff --git a/drivers/crypto/mvsam/rte_mrvl_pmd.c b/drivers/crypto/mvsam/rte_mrvl_pmd.c
index c2ae82a26..65e4769f2 100644
--- a/drivers/crypto/mvsam/rte_mrvl_pmd.c
+++ b/drivers/crypto/mvsam/rte_mrvl_pmd.c
@@ -222,6 +222,8 @@  static int
 mrvl_crypto_set_cipher_session_parameters(struct mrvl_crypto_session *sess,
 		const struct rte_crypto_sym_xform *cipher_xform)
 {
+	uint8_t *cipher_key;
+
 	/* Make sure we've got proper struct */
 	if (cipher_xform->type != RTE_CRYPTO_SYM_XFORM_CIPHER) {
 		MRVL_LOG(ERR, "Wrong xform struct provided!");
@@ -256,8 +258,17 @@  mrvl_crypto_set_cipher_session_parameters(struct mrvl_crypto_session *sess,
 		return -EINVAL;
 	}
 
+	cipher_key = malloc(cipher_xform->cipher.key.length);
+	if (cipher_key == NULL) {
+		MRVL_LOG(ERR, "Insufficient memory!");
+		return -ENOMEM;
+	}
+
+	memcpy(cipher_key, cipher_xform->cipher.key,
+			cipher_xform->cipher.key.length);
+
 	sess->sam_sess_params.cipher_key_len = cipher_xform->cipher.key.length;
-	sess->sam_sess_params.cipher_key = cipher_xform->cipher.key.data;
+	sess->sam_sess_params.cipher_key = cipher_key;
 
 	return 0;
 }
@@ -273,6 +284,8 @@  static int
 mrvl_crypto_set_auth_session_parameters(struct mrvl_crypto_session *sess,
 		const struct rte_crypto_sym_xform *auth_xform)
 {
+	uint8_t *auth_key = NULL;
+
 	/* Make sure we've got proper struct */
 	if (auth_xform->type != RTE_CRYPTO_SYM_XFORM_AUTH) {
 		MRVL_LOG(ERR, "Wrong xform struct provided!");
@@ -293,9 +306,21 @@  mrvl_crypto_set_auth_session_parameters(struct mrvl_crypto_session *sess,
 		auth_map[auth_xform->auth.algo].auth_alg;
 	sess->sam_sess_params.u.basic.auth_icv_len =
 		auth_xform->auth.digest_length;
+
+	if (auth_xform->auth.key.length > 0) {
+		auth_key = malloc(auth_xform->auth.key.length);
+		if (auth_key == NULL) {
+			MRVL_LOG(ERR, "Not enough memory!");
+			return -EINVAL;
+		}
+
+		memcpy(auth_key, auth_xform->auth.key.data,
+				auth_xform->auth.key.length);
+	}
+
+
 	/* auth_key must be NULL if auth algorithm does not use HMAC */
-	sess->sam_sess_params.auth_key = auth_xform->auth.key.length ?
-					 auth_xform->auth.key.data : NULL;
+	sess->sam_sess_params.auth_key = auth_key;
 	sess->sam_sess_params.auth_key_len = auth_xform->auth.key.length;
 
 	return 0;
@@ -312,6 +337,8 @@  static int
 mrvl_crypto_set_aead_session_parameters(struct mrvl_crypto_session *sess,
 		const struct rte_crypto_sym_xform *aead_xform)
 {
+	uint8_t *aead_key;
+
 	/* Make sure we've got proper struct */
 	if (aead_xform->type != RTE_CRYPTO_SYM_XFORM_AEAD) {
 		MRVL_LOG(ERR, "Wrong xform struct provided!");
@@ -344,7 +371,16 @@  mrvl_crypto_set_aead_session_parameters(struct mrvl_crypto_session *sess,
 		return -EINVAL;
 	}
 
-	sess->sam_sess_params.cipher_key = aead_xform->aead.key.data;
+	aead_key = malloc(aead_xform->aead.key.length);
+	if (aead_key == NULL) {
+		MRVL_LOG(ERR, "Insufficient memory!");
+		return -ENOMEM;
+	}
+
+	memcpy(aead_key, aead_xform->aead.key.data,
+			aead_xform->aead.key.length);
+
+	sess->sam_sess_params.cipher_key = aead_key;
 	sess->sam_sess_params.cipher_key_len = aead_xform->aead.key.length;
 
 	if (sess->sam_sess_params.cipher_mode == SAM_CIPHER_GCM)
diff --git a/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c b/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
index f6bf2cd4c..b334c7694 100644
--- a/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
+++ b/drivers/crypto/mvsam/rte_mrvl_pmd_ops.c
@@ -751,6 +751,8 @@  mrvl_crypto_pmd_sym_session_configure(__rte_unused struct rte_cryptodev *dev,
 		return -ENOMEM;
 	}
 
+	memset(sess_private_data, 0, sizeof(struct mrvl_crypto_session));
+
 	ret = mrvl_crypto_set_session_parameters(sess_private_data, xform);
 	if (ret != 0) {
 		MRVL_LOG(ERR, "Failed to configure session parameters!");
@@ -769,6 +771,13 @@  mrvl_crypto_pmd_sym_session_configure(__rte_unused struct rte_cryptodev *dev,
 		return -EIO;
 	}
 
+	/* free the keys memory allocated for session creation */
+	if (mrvl_sess->sam_sess_params.cipher_key != NULL)
+		free(mrvl_sess->sam_sess_params.cipher_key);
+	if (mrvl_sess->sam_sess_params.auth_key != NULL)
+		free(mrvl_sess->sam_sess_params.auth_key);
+
+
 	return 0;
 }