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

[Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>]

W dniu 23.04.2020 o 02:11, Ananyev, Konstantin pisze:
> 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?

If there is no RTE_DEBUG defined, the tested functionality is not 
compiled, so the tests won't work.

They must be wrapped with same macro as library code.

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

-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com