patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH v2] security: fix crash at accessing non-implemented ops
       [not found] <20200422235158.24497-1-konstantin.ananyev@intel.com>
@ 2020-04-23 15:10 ` Konstantin Ananyev
  2020-04-23 15:51   ` Akhil Goyal
  0 siblings, 1 reply; 5+ messages in thread
From: Konstantin Ananyev @ 2020-04-23 15:10 UTC (permalink / raw)
  To: dev; +Cc: akhil.goyal, declan.doherty, Konstantin Ananyev, stable

Valid checks for optional function pointers inside dev-ops
were disabled by undefined macro.

Fixes: b6ee98547847 ("security: fix verification of parameters")
Cc: stable@dpdk.org

Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 lib/librte_security/rte_security.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c
index d475b0977..dc9a3e89c 100644
--- a/lib/librte_security/rte_security.c
+++ b/lib/librte_security/rte_security.c
@@ -108,10 +108,11 @@ rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
 			      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);
+	RTE_PTR_OR_ERR_RET(instance, -EINVAL);
+	RTE_PTR_OR_ERR_RET(instance->ops, -EINVAL);
 #endif
+	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->set_pkt_metadata, -ENOTSUP);
 	return instance->ops->set_pkt_metadata(instance->device,
 					       sess, m, params);
 }
@@ -122,8 +123,10 @@ 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);
+	RTE_PTR_OR_ERR_RET(instance, NULL);
+	RTE_PTR_OR_ERR_RET(instance->ops, NULL);
 #endif
+	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->get_userdata, NULL);
 	if (instance->ops->get_userdata(instance->device, md, &userdata))
 		return NULL;
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-stable] [PATCH v2] security: fix crash at accessing non-implemented ops
  2020-04-23 15:10 ` [dpdk-stable] [PATCH v2] security: fix crash at accessing non-implemented ops Konstantin Ananyev
@ 2020-04-23 15:51   ` Akhil Goyal
  2020-04-23 16:08     ` Anoob Joseph
  0 siblings, 1 reply; 5+ messages in thread
From: Akhil Goyal @ 2020-04-23 15:51 UTC (permalink / raw)
  To: Konstantin Ananyev, dev, Anoob Joseph; +Cc: declan.doherty, stable


> Valid checks for optional function pointers inside dev-ops
> were disabled by undefined macro.
> 
> Fixes: b6ee98547847 ("security: fix verification of parameters")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> ---

Acked-by: Akhil Goyal <akhil.goyal@nxp.com>

Anoob,

Do you have any concerns over this patch?

Regards,
Akhil

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-stable] [PATCH v2] security: fix crash at accessing non-implemented ops
  2020-04-23 15:51   ` Akhil Goyal
@ 2020-04-23 16:08     ` Anoob Joseph
  2020-04-23 16:14       ` Akhil Goyal
  0 siblings, 1 reply; 5+ messages in thread
From: Anoob Joseph @ 2020-04-23 16:08 UTC (permalink / raw)
  To: Akhil Goyal, Konstantin Ananyev, dev; +Cc: declan.doherty, stable

Hi Akhil,

I have my concerns over unwanted checks in the datapath. Something that crypto enqueue/dequeue APIs are not doing is being enforced on other APIs. As Konstantin had suggested, PMDs (IXGBE here) could define a function which returns -ENOTSUP and it would have been win-win for everyone.

Anyway, I don't have any objections to this.

Thanks,
Anoob

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Thursday, April 23, 2020 9:22 PM
> To: Konstantin Ananyev <konstantin.ananyev@intel.com>; dev@dpdk.org;
> Anoob Joseph <anoobj@marvell.com>
> Cc: declan.doherty@intel.com; stable@dpdk.org
> Subject: [EXT] RE: [PATCH v2] security: fix crash at accessing non-implemented
> ops
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> > Valid checks for optional function pointers inside dev-ops were
> > disabled by undefined macro.
> >
> > Fixes: b6ee98547847 ("security: fix verification of parameters")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > ---
> 
> Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
> 
> Anoob,
> 
> Do you have any concerns over this patch?
> 
> Regards,
> Akhil

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-stable] [PATCH v2] security: fix crash at accessing non-implemented ops
  2020-04-23 16:08     ` Anoob Joseph
@ 2020-04-23 16:14       ` Akhil Goyal
  2020-04-23 16:29         ` [dpdk-stable] [dpdk-dev] " Lukasz Wojciechowski
  0 siblings, 1 reply; 5+ messages in thread
From: Akhil Goyal @ 2020-04-23 16:14 UTC (permalink / raw)
  To: Anoob Joseph, Konstantin Ananyev, dev; +Cc: declan.doherty, stable

Hi Anoob,
> 
> Hi Akhil,
> 
> I have my concerns over unwanted checks in the datapath. Something that
> crypto enqueue/dequeue APIs are not doing is being enforced on other APIs. As
> Konstantin had suggested, PMDs (IXGBE here) could define a function which
> returns -ENOTSUP and it would have been win-win for everyone.
> 
> Anyway, I don't have any objections to this.
> 
Your concerns are valid and those can be handled separately.
We will apply this patch.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2] security: fix crash at accessing non-implemented ops
  2020-04-23 16:14       ` Akhil Goyal
@ 2020-04-23 16:29         ` Lukasz Wojciechowski
  0 siblings, 0 replies; 5+ messages in thread
From: Lukasz Wojciechowski @ 2020-04-23 16:29 UTC (permalink / raw)
  To: Akhil Goyal, Anoob Joseph, Konstantin Ananyev, dev; +Cc: declan.doherty, stable

W dniu 23.04.2020 o 18:14, Akhil Goyal pisze:
> Hi Anoob,
>> Hi Akhil,
>>
>> I have my concerns over unwanted checks in the datapath. Something that
>> crypto enqueue/dequeue APIs are not doing is being enforced on other APIs. As
>> Konstantin had suggested, PMDs (IXGBE here) could define a function which
>> returns -ENOTSUP and it would have been win-win for everyone.
>>
>> Anyway, I don't have any objections to this.
>>
> Your concerns are valid and those can be handled separately.
> We will apply this patch.

I just pushed a patch enabling 2 tests in non-debug build mode.

Sorry for the trouble I caused


Best regards

Lukasz

-- 

Lukasz Wojciechowski
Principal Software Engineer

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-04-23 16:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200422235158.24497-1-konstantin.ananyev@intel.com>
2020-04-23 15:10 ` [dpdk-stable] [PATCH v2] security: fix crash at accessing non-implemented ops Konstantin Ananyev
2020-04-23 15:51   ` Akhil Goyal
2020-04-23 16:08     ` Anoob Joseph
2020-04-23 16:14       ` Akhil Goyal
2020-04-23 16:29         ` [dpdk-stable] [dpdk-dev] " Lukasz Wojciechowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).