Re: [dpdk-dev] [PATCH] security: fix crash at accessing non-implemented ops

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

Actually looking at app/test/test_security.c
I also see a few '#ifdef RTE_DEBUG's.
Let say:

+static int
+test_get_userdata_inv_context(void)
+{
+#ifdef RTE_DEBUG
+       uint64_t md = 0xDEADBEEF;
+
+       void *ret = rte_security_get_userdata(NULL, md);
+       TEST_ASSERT_MOCK_FUNCTION_CALL_RET(rte_security_get_userdata,
+                       ret, NULL, "%p");
+       TEST_ASSERT_MOCK_CALLS(mock_get_userdata_exp, 0);
+
+       return TEST_SUCCESS;
+#else
+       return TEST_SKIPPED;
+#endif
+}

What is the point?
Why not always run the test unconditionally?


> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Thursday, April 23, 2020 12:52 AM
> To: dev@dpdk.org
> Cc: akhil.goyal@nxp.com; Doherty, Declan <declan.doherty@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Subject: [PATCH] security: fix crash at accessing non-implemented ops
> 
> Valid checks for optional function pointers inside dev-ops
> were disabled by undefined macro.
> 
> Fixes: b6ee98547847 ("security: fix verification of parameters")
> 
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---
>  lib/librte_security/rte_security.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c
> index d475b0977..b65430ce2 100644
> --- a/lib/librte_security/rte_security.c
> +++ b/lib/librte_security/rte_security.c
> @@ -107,11 +107,9 @@ rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
>  			      struct rte_security_session *sess,
>  			      struct rte_mbuf *m, void *params)
>  {
> -#ifdef RTE_DEBUG
>  	RTE_PTR_CHAIN3_OR_ERR_RET(instance, ops, set_pkt_metadata, -EINVAL,
>  			-ENOTSUP);
>  	RTE_PTR_OR_ERR_RET(sess, -EINVAL);
> -#endif
>  	return instance->ops->set_pkt_metadata(instance->device,
>  					       sess, m, params);
>  }
> @@ -121,9 +119,7 @@ rte_security_get_userdata(struct rte_security_ctx *instance, uint64_t md)
>  {
>  	void *userdata = NULL;
> 
> -#ifdef RTE_DEBUG
>  	RTE_PTR_CHAIN3_OR_ERR_RET(instance, ops, get_userdata, NULL, NULL);
> -#endif
>  	if (instance->ops->get_userdata(instance->device, md, &userdata))
>  		return NULL;
> 
> --
> 2.17.1