DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [DPDK] net/ice: fix hash flow segmentation fault
@ 2020-03-03  1:55 taox.zhu
  2020-03-03  3:32 ` Ye Xiaolong
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: taox.zhu @ 2020-03-03  1:55 UTC (permalink / raw)
  To: qiming.yang, wenzhuo.lu
  Cc: dev, simei.su, yahui.cao, xiaolong.ye, Zhu Tao, stable

From: Zhu Tao <taox.zhu@intel.com>

Macro rte_errno is not a static value, so it needs to be updated in all
error handling code.

Patch 'dc36bd5dfd' mistakenly consider that rte_errno is a constant, which
causes the unrecognized flow rule to be marked as recognition success.
Later, when the code tried to parse the flow rule, a null pointer caused
a segmentation fault.

Fixes: dc36bd5dfd ("net/ice: fix flow FDIR/switch memory leak")
Cc: stable@dpdk.org

Signed-off-by: Zhu Tao <taox.zhu@intel.com>
---
 drivers/net/ice/ice_hash.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
index d891538bd..e5fb0f344 100644
--- a/drivers/net/ice/ice_hash.c
+++ b/drivers/net/ice/ice_hash.c
@@ -409,7 +409,7 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
 			void **meta,
 			struct rte_flow_error *error)
 {
-	int ret = -rte_errno;
+	int ret = 0;
 	struct ice_pattern_match_item *pattern_match_item;
 	struct rss_meta *rss_meta_ptr;
 
@@ -424,12 +424,16 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
 	/* Check rss supported pattern and find matched pattern. */
 	pattern_match_item = ice_search_pattern_match_item(pattern,
 					array, array_len, error);
-	if (!pattern_match_item)
+	if (!pattern_match_item) {
+		ret = -rte_errno;
 		goto error;
+	}
 
 	ret = ice_hash_check_inset(pattern, error);
-	if (ret)
+	if (ret) {
+		ret = -rte_errno;
 		goto error;
+	}
 
 	/* Save protocol header to rss_meta. */
 	*meta = rss_meta_ptr;
@@ -439,8 +443,10 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
 	/* Check rss action. */
 	ret = ice_hash_parse_action(pattern_match_item, actions, meta, error);
 error:
-	if (ret)
+	if (ret) {
+		ret = -rte_errno;
 		rte_free(rss_meta_ptr);
+	}
 	rte_free(pattern_match_item);
 
 	return ret;
-- 
2.17.1


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

* Re: [dpdk-dev] [DPDK] net/ice: fix hash flow segmentation fault
  2020-03-03  1:55 [dpdk-dev] [DPDK] net/ice: fix hash flow segmentation fault taox.zhu
@ 2020-03-03  3:32 ` Ye Xiaolong
  2020-03-03  5:07   ` Zhu, TaoX
  2020-03-03  5:07 ` [dpdk-dev] [PATCH v2] " taox.zhu
  2020-03-03  5:38 ` [dpdk-dev] [PATCH v3] " taox.zhu
  2 siblings, 1 reply; 6+ messages in thread
From: Ye Xiaolong @ 2020-03-03  3:32 UTC (permalink / raw)
  To: taox.zhu; +Cc: qiming.yang, wenzhuo.lu, dev, simei.su, yahui.cao, stable

On 03/03, taox.zhu@intel.com wrote:
>From: Zhu Tao <taox.zhu@intel.com>
>
>Macro rte_errno is not a static value, so it needs to be updated in all
>error handling code.
>
>Patch 'dc36bd5dfd' mistakenly consider that rte_errno is a constant, which
>causes the unrecognized flow rule to be marked as recognition success.
>Later, when the code tried to parse the flow rule, a null pointer caused
>a segmentation fault.
>
>Fixes: dc36bd5dfd ("net/ice: fix flow FDIR/switch memory leak")

It's recommended to have 12 chars length of commit SHA in Fixes line.
You can set below git alias for convenience.

git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae'"

>Cc: stable@dpdk.org
>
>Signed-off-by: Zhu Tao <taox.zhu@intel.com>
>---
> drivers/net/ice/ice_hash.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
>index d891538bd..e5fb0f344 100644
>--- a/drivers/net/ice/ice_hash.c
>+++ b/drivers/net/ice/ice_hash.c
>@@ -409,7 +409,7 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
> 			void **meta,
> 			struct rte_flow_error *error)
> {
>-	int ret = -rte_errno;
>+	int ret = 0;
> 	struct ice_pattern_match_item *pattern_match_item;
> 	struct rss_meta *rss_meta_ptr;
> 
>@@ -424,12 +424,16 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
> 	/* Check rss supported pattern and find matched pattern. */
> 	pattern_match_item = ice_search_pattern_match_item(pattern,
> 					array, array_len, error);
>-	if (!pattern_match_item)
>+	if (!pattern_match_item) {
>+		ret = -rte_errno;
> 		goto error;
>+	}
> 
> 	ret = ice_hash_check_inset(pattern, error);
>-	if (ret)
>+	if (ret) {
>+		ret = -rte_errno;

This seems redundant, since ice_hash_check_inset would return -rte_errno directly.

> 		goto error;
>+	}
> 
> 	/* Save protocol header to rss_meta. */
> 	*meta = rss_meta_ptr;
>@@ -439,8 +443,10 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
> 	/* Check rss action. */
> 	ret = ice_hash_parse_action(pattern_match_item, actions, meta, error);
> error:
>-	if (ret)
>+	if (ret) {
>+		ret = -rte_errno;

Ditto.

> 		rte_free(rss_meta_ptr);
>+	}
> 	rte_free(pattern_match_item);
> 
> 	return ret;
>-- 
>2.17.1
>

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

* Re: [dpdk-dev] [DPDK] net/ice: fix hash flow segmentation fault
  2020-03-03  3:32 ` Ye Xiaolong
@ 2020-03-03  5:07   ` Zhu, TaoX
  0 siblings, 0 replies; 6+ messages in thread
From: Zhu, TaoX @ 2020-03-03  5:07 UTC (permalink / raw)
  To: Ye, Xiaolong
  Cc: Yang, Qiming, Lu, Wenzhuo, dev, Su, Simei, Cao, Yahui, stable

Hi Xiaolong,

Commit SHA length non-compliance will be modified in V2 patch.
The reason why they did not return immediately after the error is that they need to release the allocated memory after the error, which is released uniformly in error processing, so they did not return directly.

BR,
Zhu, Tao


> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Tuesday, March 3, 2020 11:32 AM
> To: Zhu, TaoX <taox.zhu@intel.com>
> Cc: Yang, Qiming <qiming.yang@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; dev@dpdk.org; Su, Simei <simei.su@intel.com>;
> Cao, Yahui <yahui.cao@intel.com>; stable@dpdk.org
> Subject: Re: [DPDK] net/ice: fix hash flow segmentation fault
> 
> On 03/03, taox.zhu@intel.com wrote:
> >From: Zhu Tao <taox.zhu@intel.com>
> >
> >Macro rte_errno is not a static value, so it needs to be updated in all
> >error handling code.
> >
> >Patch 'dc36bd5dfd' mistakenly consider that rte_errno is a constant,
> >which causes the unrecognized flow rule to be marked as recognition
> success.
> >Later, when the code tried to parse the flow rule, a null pointer
> >caused a segmentation fault.
> >
> >Fixes: dc36bd5dfd ("net/ice: fix flow FDIR/switch memory leak")
> 
> It's recommended to have 12 chars length of commit SHA in Fixes line.
> You can set below git alias for convenience.
> 
> git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h
> (\"%s\")%nCc: %ae'"
> 
> >Cc: stable@dpdk.org
> >
> >Signed-off-by: Zhu Tao <taox.zhu@intel.com>
> >---
> > drivers/net/ice/ice_hash.c | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> >diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
> >index d891538bd..e5fb0f344 100644
> >--- a/drivers/net/ice/ice_hash.c
> >+++ b/drivers/net/ice/ice_hash.c
> >@@ -409,7 +409,7 @@ ice_hash_parse_pattern_action(__rte_unused
> struct ice_adapter *ad,
> > 			void **meta,
> > 			struct rte_flow_error *error)
> > {
> >-	int ret = -rte_errno;
> >+	int ret = 0;
> > 	struct ice_pattern_match_item *pattern_match_item;
> > 	struct rss_meta *rss_meta_ptr;
> >
> >@@ -424,12 +424,16 @@ ice_hash_parse_pattern_action(__rte_unused
> struct ice_adapter *ad,
> > 	/* Check rss supported pattern and find matched pattern. */
> > 	pattern_match_item = ice_search_pattern_match_item(pattern,
> > 					array, array_len, error);
> >-	if (!pattern_match_item)
> >+	if (!pattern_match_item) {
> >+		ret = -rte_errno;
> > 		goto error;
> >+	}
> >
> > 	ret = ice_hash_check_inset(pattern, error);
> >-	if (ret)
> >+	if (ret) {
> >+		ret = -rte_errno;
> 
> This seems redundant, since ice_hash_check_inset would return -rte_errno
> directly.
> 
> > 		goto error;
> >+	}
> >
> > 	/* Save protocol header to rss_meta. */
> > 	*meta = rss_meta_ptr;
> >@@ -439,8 +443,10 @@ ice_hash_parse_pattern_action(__rte_unused
> struct ice_adapter *ad,
> > 	/* Check rss action. */
> > 	ret = ice_hash_parse_action(pattern_match_item, actions, meta,
> >error);
> > error:
> >-	if (ret)
> >+	if (ret) {
> >+		ret = -rte_errno;
> 
> Ditto.
> 
> > 		rte_free(rss_meta_ptr);
> >+	}
> > 	rte_free(pattern_match_item);
> >
> > 	return ret;
> >--
> >2.17.1
> >

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

* [dpdk-dev] [PATCH v2] net/ice: fix hash flow segmentation fault
  2020-03-03  1:55 [dpdk-dev] [DPDK] net/ice: fix hash flow segmentation fault taox.zhu
  2020-03-03  3:32 ` Ye Xiaolong
@ 2020-03-03  5:07 ` taox.zhu
  2020-03-03  5:38 ` [dpdk-dev] [PATCH v3] " taox.zhu
  2 siblings, 0 replies; 6+ messages in thread
From: taox.zhu @ 2020-03-03  5:07 UTC (permalink / raw)
  To: qiming.yang, wenzhuo.lu
  Cc: dev, simei.su, yahui.cao, xiaolong.ye, Zhu Tao, stable

From: Zhu Tao <taox.zhu@intel.com>

Macro rte_errno is not a static value, so it needs to be updated in all
error handling code.

Patch 'dc36bd5dfdeb' mistakenly consider that rte_errno is a constant, which
causes the unrecognized flow rule to be marked as recognition success.
Later, when the code tried to parse the flow rule, a null pointer caused
a segmentation fault.

Fixes: dc36bd5dfdeb ("net/ice: fix flow FDIR/switch memory leak")
Cc: stable@dpdk.org

Signed-off-by: Zhu Tao <taox.zhu@intel.com>
---
v2 Changes:

 Commit message: Use 12 chars length of commit SHA in Fixes line.

 drivers/net/ice/ice_hash.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
index d891538bd..e5fb0f344 100644
--- a/drivers/net/ice/ice_hash.c
+++ b/drivers/net/ice/ice_hash.c
@@ -409,7 +409,7 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
 			void **meta,
 			struct rte_flow_error *error)
 {
-	int ret = -rte_errno;
+	int ret = 0;
 	struct ice_pattern_match_item *pattern_match_item;
 	struct rss_meta *rss_meta_ptr;
 
@@ -424,12 +424,16 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
 	/* Check rss supported pattern and find matched pattern. */
 	pattern_match_item = ice_search_pattern_match_item(pattern,
 					array, array_len, error);
-	if (!pattern_match_item)
+	if (!pattern_match_item) {
+		ret = -rte_errno;
 		goto error;
+	}
 
 	ret = ice_hash_check_inset(pattern, error);
-	if (ret)
+	if (ret) {
+		ret = -rte_errno;
 		goto error;
+	}
 
 	/* Save protocol header to rss_meta. */
 	*meta = rss_meta_ptr;
@@ -439,8 +443,10 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
 	/* Check rss action. */
 	ret = ice_hash_parse_action(pattern_match_item, actions, meta, error);
 error:
-	if (ret)
+	if (ret) {
+		ret = -rte_errno;
 		rte_free(rss_meta_ptr);
+	}
 	rte_free(pattern_match_item);
 
 	return ret;
-- 
2.17.1


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

* [dpdk-dev] [PATCH v3] net/ice: fix hash flow segmentation fault
  2020-03-03  1:55 [dpdk-dev] [DPDK] net/ice: fix hash flow segmentation fault taox.zhu
  2020-03-03  3:32 ` Ye Xiaolong
  2020-03-03  5:07 ` [dpdk-dev] [PATCH v2] " taox.zhu
@ 2020-03-03  5:38 ` taox.zhu
  2020-03-03  7:16   ` Ye Xiaolong
  2 siblings, 1 reply; 6+ messages in thread
From: taox.zhu @ 2020-03-03  5:38 UTC (permalink / raw)
  To: qiming.yang, wenzhuo.lu
  Cc: dev, simei.su, yahui.cao, xiaolong.ye, Zhu Tao, stable

From: Zhu Tao <taox.zhu@intel.com>

Macro rte_errno is not a static value, so it needs to be updated in all
error handling code.

Patch 'dc36bd5dfdeb' mistakenly consider that rte_errno is a constant,
which causes the unrecognized flow rule to be marked as recognition
success. Later, when the code tried to parse the flow rule, a null pointer
caused a segmentation fault.

Fixes: dc36bd5dfdeb ("net/ice: fix flow FDIR/switch memory leak")
Cc: stable@dpdk.org

Signed-off-by: Zhu Tao <taox.zhu@intel.com>
---
v3 Changes:

 Commit message: Use 12 chars length of commit SHA in Fixes line.
 Code: Delete redundant assignment codes.

v2 Changes:

 Commit message: Use 12 chars length of commit SHA in Fixes line.

 drivers/net/ice/ice_hash.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
index d891538bd..0e9c3c4e5 100644
--- a/drivers/net/ice/ice_hash.c
+++ b/drivers/net/ice/ice_hash.c
@@ -409,7 +409,7 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
 			void **meta,
 			struct rte_flow_error *error)
 {
-	int ret = -rte_errno;
+	int ret = 0;
 	struct ice_pattern_match_item *pattern_match_item;
 	struct rss_meta *rss_meta_ptr;
 
@@ -424,8 +424,10 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
 	/* Check rss supported pattern and find matched pattern. */
 	pattern_match_item = ice_search_pattern_match_item(pattern,
 					array, array_len, error);
-	if (!pattern_match_item)
+	if (!pattern_match_item) {
+		ret = -rte_errno;
 		goto error;
+	}
 
 	ret = ice_hash_check_inset(pattern, error);
 	if (ret)
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v3] net/ice: fix hash flow segmentation fault
  2020-03-03  5:38 ` [dpdk-dev] [PATCH v3] " taox.zhu
@ 2020-03-03  7:16   ` Ye Xiaolong
  0 siblings, 0 replies; 6+ messages in thread
From: Ye Xiaolong @ 2020-03-03  7:16 UTC (permalink / raw)
  To: taox.zhu; +Cc: qiming.yang, wenzhuo.lu, dev, simei.su, yahui.cao, stable

On 03/03, taox.zhu@intel.com wrote:
>From: Zhu Tao <taox.zhu@intel.com>
>
>Macro rte_errno is not a static value, so it needs to be updated in all
>error handling code.
>
>Patch 'dc36bd5dfdeb' mistakenly consider that rte_errno is a constant,
>which causes the unrecognized flow rule to be marked as recognition
>success. Later, when the code tried to parse the flow rule, a null pointer
>caused a segmentation fault.
>
>Fixes: dc36bd5dfdeb ("net/ice: fix flow FDIR/switch memory leak")
>Cc: stable@dpdk.org
>
>Signed-off-by: Zhu Tao <taox.zhu@intel.com>
>---
>v3 Changes:
>
> Commit message: Use 12 chars length of commit SHA in Fixes line.
> Code: Delete redundant assignment codes.
>
>v2 Changes:
>
> Commit message: Use 12 chars length of commit SHA in Fixes line.
>
> drivers/net/ice/ice_hash.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
>index d891538bd..0e9c3c4e5 100644
>--- a/drivers/net/ice/ice_hash.c
>+++ b/drivers/net/ice/ice_hash.c
>@@ -409,7 +409,7 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
> 			void **meta,
> 			struct rte_flow_error *error)
> {
>-	int ret = -rte_errno;
>+	int ret = 0;
> 	struct ice_pattern_match_item *pattern_match_item;
> 	struct rss_meta *rss_meta_ptr;
> 
>@@ -424,8 +424,10 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
> 	/* Check rss supported pattern and find matched pattern. */
> 	pattern_match_item = ice_search_pattern_match_item(pattern,
> 					array, array_len, error);
>-	if (!pattern_match_item)
>+	if (!pattern_match_item) {
>+		ret = -rte_errno;
> 		goto error;
>+	}
> 
> 	ret = ice_hash_check_inset(pattern, error);
> 	if (ret)
>-- 
>2.17.1
>

Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com>

Applied to dpdk-next-net-intel, Thanks.

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

end of thread, other threads:[~2020-03-03  7:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-03  1:55 [dpdk-dev] [DPDK] net/ice: fix hash flow segmentation fault taox.zhu
2020-03-03  3:32 ` Ye Xiaolong
2020-03-03  5:07   ` Zhu, TaoX
2020-03-03  5:07 ` [dpdk-dev] [PATCH v2] " taox.zhu
2020-03-03  5:38 ` [dpdk-dev] [PATCH v3] " taox.zhu
2020-03-03  7:16   ` Ye Xiaolong

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