[v3] linux-generic: crypto: properly handle errors in packet copy

Message ID 20170422164525.24883-1-dmitry.ereminsolenikov@linaro.org
State New
Headers show

Commit Message

Dmitry Eremin-Solenikov April 22, 2017, 4:45 p.m.
Add proper handling for errors returned by odp_packet_copy_from_pkt().

Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

---
 platform/linux-generic/odp_crypto.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

-- 
2.11.0

Comments

Balasubramanian Manoharan April 24, 2017, 5:01 a.m. | #1
Regards,
Bala


On 22 April 2017 at 22:15, Dmitry Eremin-Solenikov
<dmitry.ereminsolenikov@linaro.org> wrote:
> Add proper handling for errors returned by odp_packet_copy_from_pkt().

>

> Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>

> ---

>  platform/linux-generic/odp_crypto.c | 22 +++++++++++++++++++---

>  1 file changed, 19 insertions(+), 3 deletions(-)

>

> diff --git a/platform/linux-generic/odp_crypto.c b/platform/linux-generic/odp_crypto.c

> index 54b222fd..aaf5931d 100644

> --- a/platform/linux-generic/odp_crypto.c

> +++ b/platform/linux-generic/odp_crypto.c

> @@ -871,6 +871,7 @@ odp_crypto_operation(odp_crypto_op_param_t *param,

>         odp_crypto_alg_err_t rc_auth = ODP_CRYPTO_ALG_ERR_NONE;

>         odp_crypto_generic_session_t *session;

>         odp_crypto_op_result_t local_result;

> +       odp_bool_t allocated = false;

>

>         session = (odp_crypto_generic_session_t *)(intptr_t)param->session;

>

> @@ -885,12 +886,19 @@ odp_crypto_operation(odp_crypto_op_param_t *param,

>                 return -1;

>         }

>

> +       allocated = true;


This boolean 'allocated' should come inside the if condition where you
allocate the packet.
During "in place" encryption the out_pkt is same as input packet in
which case the odp_crypto_operation should not free the packet during
error. Since in that case the packet was allocated by the application
it may need to resubmit the packet for further crypto operation.

> +

>         if (param->pkt != param->out_pkt) {

> -               (void)odp_packet_copy_from_pkt(param->out_pkt,

> +               int ret;

> +

> +               ret = odp_packet_copy_from_pkt(param->out_pkt,

>                                                0,

>                                                param->pkt,

>                                                0,

>                                                odp_packet_len(param->pkt));

> +               if (odp_unlikely(ret < 0))

> +                       goto err;

> +

>                 _odp_packet_copy_md_to_packet(param->pkt, param->out_pkt);

>                 odp_packet_free(param->pkt);

>                 param->pkt = ODP_PACKET_INVALID;

> @@ -932,7 +940,7 @@ odp_crypto_operation(odp_crypto_op_param_t *param,

>                 op_result->result = local_result;

>                 if (odp_queue_enq(session->p.compl_queue, completion_event)) {

>                         odp_event_free(completion_event);

> -                       return -1;

> +                       goto err;

>                 }

>

>                 /* Indicate to caller operation was async */

> @@ -940,13 +948,21 @@ odp_crypto_operation(odp_crypto_op_param_t *param,

>         } else {

>                 /* Synchronous, simply return results */

>                 if (!result)

> -                       return -1;

> +                       goto err;

>                 *result = local_result;

>

>                 /* Indicate to caller operation was sync */

>                 *posted = 0;

>         }

>         return 0;

> +

> +err:

> +       if (allocated) {

> +               odp_packet_free(param->out_pkt);

> +               param->out_pkt = ODP_PACKET_INVALID;

> +       }

> +

> +       return -1;

>  }

>

>  static void ODP_UNUSED openssl_thread_id(CRYPTO_THREADID ODP_UNUSED *id)

> --

> 2.11.0

>

Patch

diff --git a/platform/linux-generic/odp_crypto.c b/platform/linux-generic/odp_crypto.c
index 54b222fd..aaf5931d 100644
--- a/platform/linux-generic/odp_crypto.c
+++ b/platform/linux-generic/odp_crypto.c
@@ -871,6 +871,7 @@  odp_crypto_operation(odp_crypto_op_param_t *param,
 	odp_crypto_alg_err_t rc_auth = ODP_CRYPTO_ALG_ERR_NONE;
 	odp_crypto_generic_session_t *session;
 	odp_crypto_op_result_t local_result;
+	odp_bool_t allocated = false;
 
 	session = (odp_crypto_generic_session_t *)(intptr_t)param->session;
 
@@ -885,12 +886,19 @@  odp_crypto_operation(odp_crypto_op_param_t *param,
 		return -1;
 	}
 
+	allocated = true;
+
 	if (param->pkt != param->out_pkt) {
-		(void)odp_packet_copy_from_pkt(param->out_pkt,
+		int ret;
+
+		ret = odp_packet_copy_from_pkt(param->out_pkt,
 					       0,
 					       param->pkt,
 					       0,
 					       odp_packet_len(param->pkt));
+		if (odp_unlikely(ret < 0))
+			goto err;
+
 		_odp_packet_copy_md_to_packet(param->pkt, param->out_pkt);
 		odp_packet_free(param->pkt);
 		param->pkt = ODP_PACKET_INVALID;
@@ -932,7 +940,7 @@  odp_crypto_operation(odp_crypto_op_param_t *param,
 		op_result->result = local_result;
 		if (odp_queue_enq(session->p.compl_queue, completion_event)) {
 			odp_event_free(completion_event);
-			return -1;
+			goto err;
 		}
 
 		/* Indicate to caller operation was async */
@@ -940,13 +948,21 @@  odp_crypto_operation(odp_crypto_op_param_t *param,
 	} else {
 		/* Synchronous, simply return results */
 		if (!result)
-			return -1;
+			goto err;
 		*result = local_result;
 
 		/* Indicate to caller operation was sync */
 		*posted = 0;
 	}
 	return 0;
+
+err:
+	if (allocated) {
+		odp_packet_free(param->out_pkt);
+		param->out_pkt = ODP_PACKET_INVALID;
+	}
+
+	return -1;
 }
 
 static void ODP_UNUSED openssl_thread_id(CRYPTO_THREADID ODP_UNUSED *id)