diff mbox

[v1] crypto : use override_iv_ptr when session IV is NULL

Message ID 1416392862-6874-1-git-send-email-alexandru.badicioiu@linaro.org
State Accepted
Commit ca94d16dbb258e44da7b9ccfbeecb25ae2a7afd2
Headers show

Commit Message

Alexandru Badicioiu Nov. 19, 2014, 10:27 a.m. UTC 
From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>

NULL session IV prevents using per operation IV (override_iv_ptr).
This fixes *_OVR_IV cunit tests segfaults.

Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
---
 platform/linux-generic/odp_crypto.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

Comments

Maxim Uvarov Nov. 20, 2014, 8:31 a.m. UTC | #1 
Looks  reasonable. One more review please.

Maxim.

On 11/19/2014 01:27 PM, alexandru.badicioiu@linaro.org wrote:
> From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
>
> NULL session IV prevents using per operation IV (override_iv_ptr).
> This fixes *_OVR_IV cunit tests segfaults.
>
> Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
> ---
>   platform/linux-generic/odp_crypto.c |   16 +++++++++-------
>   1 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/platform/linux-generic/odp_crypto.c b/platform/linux-generic/odp_crypto.c
> index 596c717..9ca2251 100644
> --- a/platform/linux-generic/odp_crypto.c
> +++ b/platform/linux-generic/odp_crypto.c
> @@ -147,7 +147,7 @@ enum crypto_alg_err des_encrypt(odp_crypto_op_params_t *params,
>   {
>   	uint8_t *data  = odp_packet_addr(params->out_pkt);
>   	uint32_t len   = params->cipher_range.length;
> -	DES_cblock *iv;
> +	DES_cblock *iv = NULL;
>   	DES_cblock iv_temp;
>   
>   	/*
> @@ -155,8 +155,10 @@ enum crypto_alg_err des_encrypt(odp_crypto_op_params_t *params,
>   	 * and if we are processing packets on parallel threads
>   	 * we could get corruption.
>   	 */
> -	memcpy(iv_temp, session->cipher.iv.data, sizeof(iv_temp));
> -	iv = &iv_temp;
> +	if (session->cipher.iv.data) {
> +		memcpy(iv_temp, session->cipher.iv.data, sizeof(iv_temp));
> +		iv = &iv_temp;
> +	}
>   
>   	/* Adjust pointer for beginning of area to cipher */
>   	data += params->cipher_range.offset;
> @@ -165,6 +167,10 @@ enum crypto_alg_err des_encrypt(odp_crypto_op_params_t *params,
>   	if (params->override_iv_ptr)
>   		iv = (DES_cblock *)params->override_iv_ptr;
>   
> +	/* No session or operation IV */
> +	if (!iv)
> +		return ODP_CRYPTO_SES_CREATE_ERR_INV_CIPHER;
> +
>   	/* Encrypt it */
>   	DES_ede3_cbc_encrypt(data,
>   			     data,
> @@ -214,10 +220,6 @@ int process_des_params(odp_crypto_generic_session_t *session,
>   	if (!((0 == params->iv.length) || (8 == params->iv.length)))
>   		return -1;
>   
> -	/* Verify IV pointer */
> -	if (params->iv.length && !params->iv.data)
> -		return -1;
> -
>   	/* Set function */
>   	if (ODP_CRYPTO_OP_ENCODE == params->op)
>   		session->cipher.func = des_encrypt;
Anders Roxell Nov. 20, 2014, 9:13 a.m. UTC | #2 
On 2014-11-19 12:27, alexandru.badicioiu@linaro.org wrote:
> From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
> 
> NULL session IV prevents using per operation IV (override_iv_ptr).
> This fixes *_OVR_IV cunit tests segfaults.
> 
> Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>

Reviewed-and-Tested-by: Anders Roxell <anders.roxell@linaro.org>

> ---
>  platform/linux-generic/odp_crypto.c |   16 +++++++++-------
>  1 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/platform/linux-generic/odp_crypto.c b/platform/linux-generic/odp_crypto.c
> index 596c717..9ca2251 100644
> --- a/platform/linux-generic/odp_crypto.c
> +++ b/platform/linux-generic/odp_crypto.c
> @@ -147,7 +147,7 @@ enum crypto_alg_err des_encrypt(odp_crypto_op_params_t *params,
>  {
>  	uint8_t *data  = odp_packet_addr(params->out_pkt);
>  	uint32_t len   = params->cipher_range.length;
> -	DES_cblock *iv;
> +	DES_cblock *iv = NULL;
>  	DES_cblock iv_temp;
>  
>  	/*
> @@ -155,8 +155,10 @@ enum crypto_alg_err des_encrypt(odp_crypto_op_params_t *params,
>  	 * and if we are processing packets on parallel threads
>  	 * we could get corruption.
>  	 */
> -	memcpy(iv_temp, session->cipher.iv.data, sizeof(iv_temp));
> -	iv = &iv_temp;
> +	if (session->cipher.iv.data) {
> +		memcpy(iv_temp, session->cipher.iv.data, sizeof(iv_temp));
> +		iv = &iv_temp;
> +	}
>  
>  	/* Adjust pointer for beginning of area to cipher */
>  	data += params->cipher_range.offset;
> @@ -165,6 +167,10 @@ enum crypto_alg_err des_encrypt(odp_crypto_op_params_t *params,
>  	if (params->override_iv_ptr)
>  		iv = (DES_cblock *)params->override_iv_ptr;
>  
> +	/* No session or operation IV */
> +	if (!iv)
> +		return ODP_CRYPTO_SES_CREATE_ERR_INV_CIPHER;
> +
>  	/* Encrypt it */
>  	DES_ede3_cbc_encrypt(data,
>  			     data,
> @@ -214,10 +220,6 @@ int process_des_params(odp_crypto_generic_session_t *session,
>  	if (!((0 == params->iv.length) || (8 == params->iv.length)))
>  		return -1;
>  
> -	/* Verify IV pointer */
> -	if (params->iv.length && !params->iv.data)
> -		return -1;
> -
>  	/* Set function */
>  	if (ODP_CRYPTO_OP_ENCODE == params->op)
>  		session->cipher.func = des_encrypt;
> -- 
> 1.7.3.4
> 
> 
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
Mike Holmes Nov. 20, 2014, 12:56 p.m. UTC | #3 
On 20 November 2014 04:13, Anders Roxell <anders.roxell@linaro.org> wrote:

> On 2014-11-19 12:27, alexandru.badicioiu@linaro.org wrote:
> > From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
> >
> > NULL session IV prevents using per operation IV (override_iv_ptr).
> > This fixes *_OVR_IV cunit tests segfaults.
> >
> > Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
>
> Reviewed-and-Tested-by: Anders Roxell <anders.roxell@linaro.org>
>
 Reviewed-and-Tested-by:: Mike Holmes <mike.holmes@linaro.org>

This does fix the crypto test sag fault found with the v8 crypto test patch.


> > ---
> >  platform/linux-generic/odp_crypto.c |   16 +++++++++-------
> >  1 files changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/platform/linux-generic/odp_crypto.c
> b/platform/linux-generic/odp_crypto.c
> > index 596c717..9ca2251 100644
> > --- a/platform/linux-generic/odp_crypto.c
> > +++ b/platform/linux-generic/odp_crypto.c
> > @@ -147,7 +147,7 @@ enum crypto_alg_err
> des_encrypt(odp_crypto_op_params_t *params,
> >  {
> >       uint8_t *data  = odp_packet_addr(params->out_pkt);
> >       uint32_t len   = params->cipher_range.length;
> > -     DES_cblock *iv;
> > +     DES_cblock *iv = NULL;
> >       DES_cblock iv_temp;
> >
> >       /*
> > @@ -155,8 +155,10 @@ enum crypto_alg_err
> des_encrypt(odp_crypto_op_params_t *params,
> >        * and if we are processing packets on parallel threads
> >        * we could get corruption.
> >        */
> > -     memcpy(iv_temp, session->cipher.iv.data, sizeof(iv_temp));
> > -     iv = &iv_temp;
> > +     if (session->cipher.iv.data) {
> > +             memcpy(iv_temp, session->cipher.iv.data, sizeof(iv_temp));
> > +             iv = &iv_temp;
> > +     }
> >
> >       /* Adjust pointer for beginning of area to cipher */
> >       data += params->cipher_range.offset;
> > @@ -165,6 +167,10 @@ enum crypto_alg_err
> des_encrypt(odp_crypto_op_params_t *params,
> >       if (params->override_iv_ptr)
> >               iv = (DES_cblock *)params->override_iv_ptr;
> >
> > +     /* No session or operation IV */
> > +     if (!iv)
> > +             return ODP_CRYPTO_SES_CREATE_ERR_INV_CIPHER;
> > +
> >       /* Encrypt it */
> >       DES_ede3_cbc_encrypt(data,
> >                            data,
> > @@ -214,10 +220,6 @@ int process_des_params(odp_crypto_generic_session_t
> *session,
> >       if (!((0 == params->iv.length) || (8 == params->iv.length)))
> >               return -1;
> >
> > -     /* Verify IV pointer */
> > -     if (params->iv.length && !params->iv.data)
> > -             return -1;
> > -
> >       /* Set function */
> >       if (ODP_CRYPTO_OP_ENCODE == params->op)
> >               session->cipher.func = des_encrypt;
> > --
> > 1.7.3.4
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Maxim Uvarov Nov. 20, 2014, 2:23 p.m. UTC | #4 
Merged,

Thanks,
Maxim.

On 11/19/2014 01:27 PM, alexandru.badicioiu@linaro.org wrote:
> From: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
>
> NULL session IV prevents using per operation IV (override_iv_ptr).
> This fixes *_OVR_IV cunit tests segfaults.
>
> Signed-off-by: Alexandru Badicioiu <alexandru.badicioiu@linaro.org>
> ---
>   platform/linux-generic/odp_crypto.c |   16 +++++++++-------
>   1 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/platform/linux-generic/odp_crypto.c b/platform/linux-generic/odp_crypto.c
> index 596c717..9ca2251 100644
> --- a/platform/linux-generic/odp_crypto.c
> +++ b/platform/linux-generic/odp_crypto.c
> @@ -147,7 +147,7 @@ enum crypto_alg_err des_encrypt(odp_crypto_op_params_t *params,
>   {
>   	uint8_t *data  = odp_packet_addr(params->out_pkt);
>   	uint32_t len   = params->cipher_range.length;
> -	DES_cblock *iv;
> +	DES_cblock *iv = NULL;
>   	DES_cblock iv_temp;
>   
>   	/*
> @@ -155,8 +155,10 @@ enum crypto_alg_err des_encrypt(odp_crypto_op_params_t *params,
>   	 * and if we are processing packets on parallel threads
>   	 * we could get corruption.
>   	 */
> -	memcpy(iv_temp, session->cipher.iv.data, sizeof(iv_temp));
> -	iv = &iv_temp;
> +	if (session->cipher.iv.data) {
> +		memcpy(iv_temp, session->cipher.iv.data, sizeof(iv_temp));
> +		iv = &iv_temp;
> +	}
>   
>   	/* Adjust pointer for beginning of area to cipher */
>   	data += params->cipher_range.offset;
> @@ -165,6 +167,10 @@ enum crypto_alg_err des_encrypt(odp_crypto_op_params_t *params,
>   	if (params->override_iv_ptr)
>   		iv = (DES_cblock *)params->override_iv_ptr;
>   
> +	/* No session or operation IV */
> +	if (!iv)
> +		return ODP_CRYPTO_SES_CREATE_ERR_INV_CIPHER;
> +
>   	/* Encrypt it */
>   	DES_ede3_cbc_encrypt(data,
>   			     data,
> @@ -214,10 +220,6 @@ int process_des_params(odp_crypto_generic_session_t *session,
>   	if (!((0 == params->iv.length) || (8 == params->iv.length)))
>   		return -1;
>   
> -	/* Verify IV pointer */
> -	if (params->iv.length && !params->iv.data)
> -		return -1;
> -
>   	/* Set function */
>   	if (ODP_CRYPTO_OP_ENCODE == params->op)
>   		session->cipher.func = des_encrypt;
diff mbox

Patch

diff --git a/platform/linux-generic/odp_crypto.c b/platform/linux-generic/odp_crypto.c
index 596c717..9ca2251 100644
--- a/platform/linux-generic/odp_crypto.c
+++ b/platform/linux-generic/odp_crypto.c
@@ -147,7 +147,7 @@  enum crypto_alg_err des_encrypt(odp_crypto_op_params_t *params,
 {
 	uint8_t *data  = odp_packet_addr(params->out_pkt);
 	uint32_t len   = params->cipher_range.length;
-	DES_cblock *iv;
+	DES_cblock *iv = NULL;
 	DES_cblock iv_temp;
 
 	/*
@@ -155,8 +155,10 @@  enum crypto_alg_err des_encrypt(odp_crypto_op_params_t *params,
 	 * and if we are processing packets on parallel threads
 	 * we could get corruption.
 	 */
-	memcpy(iv_temp, session->cipher.iv.data, sizeof(iv_temp));
-	iv = &iv_temp;
+	if (session->cipher.iv.data) {
+		memcpy(iv_temp, session->cipher.iv.data, sizeof(iv_temp));
+		iv = &iv_temp;
+	}
 
 	/* Adjust pointer for beginning of area to cipher */
 	data += params->cipher_range.offset;
@@ -165,6 +167,10 @@  enum crypto_alg_err des_encrypt(odp_crypto_op_params_t *params,
 	if (params->override_iv_ptr)
 		iv = (DES_cblock *)params->override_iv_ptr;
 
+	/* No session or operation IV */
+	if (!iv)
+		return ODP_CRYPTO_SES_CREATE_ERR_INV_CIPHER;
+
 	/* Encrypt it */
 	DES_ede3_cbc_encrypt(data,
 			     data,
@@ -214,10 +220,6 @@  int process_des_params(odp_crypto_generic_session_t *session,
 	if (!((0 == params->iv.length) || (8 == params->iv.length)))
 		return -1;
 
-	/* Verify IV pointer */
-	if (params->iv.length && !params->iv.data)
-		return -1;
-
 	/* Set function */
 	if (ODP_CRYPTO_OP_ENCODE == params->op)
 		session->cipher.func = des_encrypt;