DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
@ 2020-04-30 12:05 yuanlinsi01
  2020-04-30 12:55 ` Somnath Kotur
  0 siblings, 1 reply; 14+ messages in thread
From: yuanlinsi01 @ 2020-04-30 12:05 UTC (permalink / raw)
  To: ajit.khaparde, somnath.kotur; +Cc: dev

We see a stack smashing as a result of defensive code missing. Once the
nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
zero after doing a floor align, and we can not exit the following
receiving packets loop. And the buffers will be overwrite, then the
stack frame was ruined.

Fix the problem by adding defensive code, once the nb_pkts is zero, just
directly return with no packets.

Signed-off-by: yuanlinsi01 <yuanlinsi01@baidu.com>
Signed-off-by: rongdongsheng <rongdongsheng@baidu.com>
---
 drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
index d0e7910e7..c4adccdbc 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
@@ -233,8 +233,13 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	/* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
 	nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
 
-	/* Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP */
+	/*
+	 * Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP
+	 * nb_pkts < RTE_BNXT_DESCS_PER_LOOP, just return no packet
+	 */
 	nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_BNXT_DESCS_PER_LOOP);
+	if (!nb_pkts)
+		return 0;
 
 	/* Handle RX burst request */
 	while (1) {
-- 
2.11.0


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

* Re: [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
  2020-04-30 12:05 [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing yuanlinsi01
@ 2020-04-30 12:55 ` Somnath Kotur
  2020-04-30 13:33   ` [dpdk-dev] 答复: " Yuan,Linsi
  0 siblings, 1 reply; 14+ messages in thread
From: Somnath Kotur @ 2020-04-30 12:55 UTC (permalink / raw)
  To: yuanlinsi01, Lance Richardson; +Cc: Ajit Kumar Khaparde, dev

+Lance Richardson

Thanks for the patch, could you please add the 'Fixes' tag as well ?



On Thu, Apr 30, 2020 at 5:35 PM yuanlinsi01 <yuanlinsi01@baidu.com> wrote:
>
> We see a stack smashing as a result of defensive code missing. Once the
> nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
> zero after doing a floor align, and we can not exit the following
> receiving packets loop. And the buffers will be overwrite, then the
> stack frame was ruined.
>
> Fix the problem by adding defensive code, once the nb_pkts is zero, just
> directly return with no packets.
>
> Signed-off-by: yuanlinsi01 <yuanlinsi01@baidu.com>
> Signed-off-by: rongdongsheng <rongdongsheng@baidu.com>
> ---
>  drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> index d0e7910e7..c4adccdbc 100644
> --- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> +++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> @@ -233,8 +233,13 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
>         /* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
>         nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
>
> -       /* Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP */
> +       /*
> +        * Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP
> +        * nb_pkts < RTE_BNXT_DESCS_PER_LOOP, just return no packet
> +        */
>         nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_BNXT_DESCS_PER_LOOP);
> +       if (!nb_pkts)
> +               return 0;
>
>         /* Handle RX burst request */
>         while (1) {
> --
> 2.11.0
>

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

* [dpdk-dev] 答复: [PATCH] net/bnxt: fix a possible stack smashing
  2020-04-30 12:55 ` Somnath Kotur
@ 2020-04-30 13:33   ` Yuan,Linsi
  0 siblings, 0 replies; 14+ messages in thread
From: Yuan,Linsi @ 2020-04-30 13:33 UTC (permalink / raw)
  To: Somnath Kotur, Lance Richardson; +Cc: Ajit Kumar Khaparde, dev

Sure, I'll add it.


Thanks,

Yuan Linsi

________________________________
发件人: Somnath Kotur <somnath.kotur@broadcom.com>
发送时间: 2020年4月30日 20:55
收件人: Yuan,Linsi; Lance Richardson
抄送: Ajit Kumar Khaparde; dev
主题: Re: [PATCH] net/bnxt: fix a possible stack smashing

+Lance Richardson

Thanks for the patch, could you please add the 'Fixes' tag as well ?



On Thu, Apr 30, 2020 at 5:35 PM yuanlinsi01 <yuanlinsi01@baidu.com> wrote:
>
> We see a stack smashing as a result of defensive code missing. Once the
> nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
> zero after doing a floor align, and we can not exit the following
> receiving packets loop. And the buffers will be overwrite, then the
> stack frame was ruined.
>
> Fix the problem by adding defensive code, once the nb_pkts is zero, just
> directly return with no packets.
>
> Signed-off-by: yuanlinsi01 <yuanlinsi01@baidu.com>
> Signed-off-by: rongdongsheng <rongdongsheng@baidu.com>
> ---
>  drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> index d0e7910e7..c4adccdbc 100644
> --- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> +++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> @@ -233,8 +233,13 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
>         /* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
>         nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
>
> -       /* Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP */
> +       /*
> +        * Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP
> +        * nb_pkts < RTE_BNXT_DESCS_PER_LOOP, just return no packet
> +        */
>         nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_BNXT_DESCS_PER_LOOP);
> +       if (!nb_pkts)
> +               return 0;
>
>         /* Handle RX burst request */
>         while (1) {
> --
> 2.11.0
>

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

* Re: [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
  2020-05-06  3:28 [dpdk-dev] " Yuan Linsi
@ 2020-05-06  5:26 ` Ajit Khaparde
  0 siblings, 0 replies; 14+ messages in thread
From: Ajit Khaparde @ 2020-05-06  5:26 UTC (permalink / raw)
  To: Yuan Linsi; +Cc: Somnath Kotur, Lance Richardson, dpdk-dev

On Tue, May 5, 2020 at 8:29 PM Yuan Linsi <yuanlinsi01@baidu.com> wrote:

> From: Linsi Yuan <yuanlinsi01@baidu.com>
>
> We see a stack smashing as a result of defensive code missing. Once the
> nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
> zero after doing a floor align, and we can not exit the following
> receiving packets loop. And the buffers will be overwrite, then the
> stack frame was ruined.
>
> Fix the problem by adding defensive code, once the nb_pkts is zero, just
> directly return with no packets.
>
> Fixes: bc4a000f2 ("net/bnxt: implement SSE vector mode")
> Cc: stable@dpdk.org
>
> Signed-off-by: Linsi Yuan <yuanlinsi01@baidu.com>
> Signed-off-by: Dongsheng Rong <rongdongsheng@baidu.com>
>
Thanks. I updated the earlier version [1] with this Signed-off.

[1] https://patchwork.dpdk.org/patch/69604/


> ---
>  drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> index d0e7910e7..8f73add9b 100644
> --- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> +++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> @@ -233,8 +233,13 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf
> **rx_pkts,
>         /* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
>         nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
>
> -       /* Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP */
> +       /*
> +        * Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP.
> +        * nb_pkts < RTE_BNXT_DESCS_PER_LOOP, just return no packet
> +        */
>         nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_BNXT_DESCS_PER_LOOP);
> +       if (!nb_pkts)
> +               return 0;
>
>         /* Handle RX burst request */
>         while (1) {
> --
> 2.11.0
>
>

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

* [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
@ 2020-05-06  3:28 Yuan Linsi
  2020-05-06  5:26 ` Ajit Khaparde
  0 siblings, 1 reply; 14+ messages in thread
From: Yuan Linsi @ 2020-05-06  3:28 UTC (permalink / raw)
  To: ajit.khaparde, somnath.kotur, lance.richardson; +Cc: dev

From: Linsi Yuan <yuanlinsi01@baidu.com>

We see a stack smashing as a result of defensive code missing. Once the
nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
zero after doing a floor align, and we can not exit the following
receiving packets loop. And the buffers will be overwrite, then the
stack frame was ruined.

Fix the problem by adding defensive code, once the nb_pkts is zero, just
directly return with no packets.

Fixes: bc4a000f2 ("net/bnxt: implement SSE vector mode")
Cc: stable@dpdk.org

Signed-off-by: Linsi Yuan <yuanlinsi01@baidu.com>
Signed-off-by: Dongsheng Rong <rongdongsheng@baidu.com>
---
 drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
index d0e7910e7..8f73add9b 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
@@ -233,8 +233,13 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	/* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
 	nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
 
-	/* Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP */
+	/*
+	 * Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP.
+	 * nb_pkts < RTE_BNXT_DESCS_PER_LOOP, just return no packet
+	 */
 	nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_BNXT_DESCS_PER_LOOP);
+	if (!nb_pkts)
+		return 0;
 
 	/* Handle RX burst request */
 	while (1) {
-- 
2.11.0


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

* [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
@ 2020-05-06  3:18 Yuan Linsi
  0 siblings, 0 replies; 14+ messages in thread
From: Yuan Linsi @ 2020-05-06  3:18 UTC (permalink / raw)
  To: ajit.khaparde, somnath.kotur, lance.richardson; +Cc: dev

From: yuanlinsi01 <yuanlinsi01@baidu.com>

We see a stack smashing as a result of defensive code missing. Once the
nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
zero after doing a floor align, and we can not exit the following
receiving packets loop. And the buffers will be overwrite, then the
stack frame was ruined.

Fix the problem by adding defensive code, once the nb_pkts is zero, just
directly return with no packets.

Fixes: bc4a000f2 ("net/bnxt: implement SSE vector mode")
Cc: stable@dpdk.org

Signed-off-by: Linsi Yuan <yuanlinsi01@baidu.com>
Signed-off-by: Dongsheng Rong <rongdongsheng@baidu.com>
---
 drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
index d0e7910e7..8f73add9b 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
@@ -233,8 +233,13 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	/* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
 	nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
 
-	/* Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP */
+	/*
+	 * Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP.
+	 * nb_pkts < RTE_BNXT_DESCS_PER_LOOP, just return no packet
+	 */
 	nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_BNXT_DESCS_PER_LOOP);
+	if (!nb_pkts)
+		return 0;
 
 	/* Handle RX burst request */
 	while (1) {
-- 
2.11.0


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

* [dpdk-dev] 答复:  [PATCH] net/bnxt: fix a possible stack smashing
  2020-05-05  3:42   ` Ajit Khaparde
@ 2020-05-06  3:18     ` Yuan,Linsi
  0 siblings, 0 replies; 14+ messages in thread
From: Yuan,Linsi @ 2020-05-06  3:18 UTC (permalink / raw)
  To: Ajit Khaparde, Ferruh Yigit; +Cc: Somnath Kotur, Lance Richardson, dpdk-dev

Ok, I'll modify it per you suggestion

Thanks,
Yuan Linsi
________________________________
发件人: Ajit Khaparde <ajit.khaparde@broadcom.com>
发送时间: 2020年5月5日 11:42:20
收件人: Ferruh Yigit
抄送: Yuan,Linsi; Somnath Kotur; Lance Richardson; dpdk-dev
主题: Re: [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing



On Thu, Apr 30, 2020 at 4:55 PM Ferruh Yigit <ferruh.yigit@intel.com<mailto:ferruh.yigit@intel.com>> wrote:
On 4/30/2020 2:37 PM, Yuan Linsi wrote:
> From: yuanlinsi01 <yuanlinsi01@baidu.com<mailto:yuanlinsi01@baidu.com>>
>
> We see a stack smashing as a result of defensive code missing. Once the
> nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
> zero after doing a floor align, and we can not exit the following
> receiving packets loop. And the buffers will be overwrite, then the
> stack frame was ruined.
>
> Fix the problem by adding defensive code, once the nb_pkts is zero, just
> directly return with no packets.
>
> Fixes: bc4a000f2 ("net/bnxt: implement SSE vector mode")
> Cc: stable@dpdk.org<mailto:stable@dpdk.org>
>
> Signed-off-by: yuanlinsi01 <yuanlinsi01@baidu.com<mailto:yuanlinsi01@baidu.com>>
> Signed-off-by: rongdongsheng <rongdongsheng@baidu.com<mailto:rongdongsheng@baidu.com>>

Hi Yuan,

For the sign-off tag, we need "Name Surname <email@adress.com<mailto:email@adress.com>>" syntax,
for you I can see from mail thread that it is:
 "Signed-off-by: Linsi Yuan <yuanlinsi01@baidu.com<mailto:yuanlinsi01@baidu.com>>"

Can you please share the same for the other sign-off, 'rongdongsheng'?
Yuan, Can you please send the proper first-name, last-name for rongdongsheng? Thanks

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

* Re: [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
  2020-04-30 23:55 ` Ferruh Yigit
@ 2020-05-05  3:42   ` Ajit Khaparde
  2020-05-06  3:18     ` [dpdk-dev] 答复: " Yuan,Linsi
  0 siblings, 1 reply; 14+ messages in thread
From: Ajit Khaparde @ 2020-05-05  3:42 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Yuan Linsi, Somnath Kotur, Lance Richardson, dpdk-dev

On Thu, Apr 30, 2020 at 4:55 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 4/30/2020 2:37 PM, Yuan Linsi wrote:
> > From: yuanlinsi01 <yuanlinsi01@baidu.com>
> >
> > We see a stack smashing as a result of defensive code missing. Once the
> > nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
> > zero after doing a floor align, and we can not exit the following
> > receiving packets loop. And the buffers will be overwrite, then the
> > stack frame was ruined.
> >
> > Fix the problem by adding defensive code, once the nb_pkts is zero, just
> > directly return with no packets.
> >
> > Fixes: bc4a000f2 ("net/bnxt: implement SSE vector mode")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: yuanlinsi01 <yuanlinsi01@baidu.com>
> > Signed-off-by: rongdongsheng <rongdongsheng@baidu.com>
>
> Hi Yuan,
>
> For the sign-off tag, we need "Name Surname <email@adress.com>" syntax,
> for you I can see from mail thread that it is:
>  "Signed-off-by: Linsi Yuan <yuanlinsi01@baidu.com>"
>
> Can you please share the same for the other sign-off, 'rongdongsheng'?
>
Yuan, Can you please send the proper first-name, last-name
for rongdongsheng? Thanks

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

* Re: [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
  2020-04-30 13:37 Yuan Linsi
  2020-04-30 13:45 ` Lance Richardson
@ 2020-04-30 23:55 ` Ferruh Yigit
  2020-05-05  3:42   ` Ajit Khaparde
  1 sibling, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2020-04-30 23:55 UTC (permalink / raw)
  To: Yuan Linsi, ajit.khaparde, somnath.kotur, lance.richardson; +Cc: dev

On 4/30/2020 2:37 PM, Yuan Linsi wrote:
> From: yuanlinsi01 <yuanlinsi01@baidu.com>
> 
> We see a stack smashing as a result of defensive code missing. Once the
> nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
> zero after doing a floor align, and we can not exit the following
> receiving packets loop. And the buffers will be overwrite, then the
> stack frame was ruined.
> 
> Fix the problem by adding defensive code, once the nb_pkts is zero, just
> directly return with no packets.
> 
> Fixes: bc4a000f2 ("net/bnxt: implement SSE vector mode")
> Cc: stable@dpdk.org
> 
> Signed-off-by: yuanlinsi01 <yuanlinsi01@baidu.com>
> Signed-off-by: rongdongsheng <rongdongsheng@baidu.com>

Hi Yuan,

For the sign-off tag, we need "Name Surname <email@adress.com>" syntax,
for you I can see from mail thread that it is:
 "Signed-off-by: Linsi Yuan <yuanlinsi01@baidu.com>"

Can you please share the same for the other sign-off, 'rongdongsheng'?


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

* Re: [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
  2020-04-30 13:45 ` Lance Richardson
@ 2020-04-30 18:29   ` Ajit Khaparde
  0 siblings, 0 replies; 14+ messages in thread
From: Ajit Khaparde @ 2020-04-30 18:29 UTC (permalink / raw)
  To: Lance Richardson; +Cc: Yuan Linsi, Somnath Kotur, dpdk-dev

On Thu, Apr 30, 2020 at 6:45 AM Lance Richardson <
lance.richardson@broadcom.com> wrote:

> On Thu, Apr 30, 2020 at 9:37 AM Yuan Linsi <yuanlinsi01@baidu.com> wrote:
> >
> > From: yuanlinsi01 <yuanlinsi01@baidu.com>
> >
> > We see a stack smashing as a result of defensive code missing. Once the
> > nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
> > zero after doing a floor align, and we can not exit the following
> > receiving packets loop. And the buffers will be overwrite, then the
> > stack frame was ruined.
> >
> > Fix the problem by adding defensive code, once the nb_pkts is zero, just
> > directly return with no packets.
> >
> > Fixes: bc4a000f2 ("net/bnxt: implement SSE vector mode")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: yuanlinsi01 <yuanlinsi01@baidu.com>
> > Signed-off-by: rongdongsheng <rongdongsheng@baidu.com>
>
> Thanks for the fix!
>
> Acked-by: Lance Richardson <lance.richardson@broadcom.com>
>
Patch applied to dpdk-next-net-brcm.  Thanks

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

* Re: [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
  2020-04-30 13:37 Yuan Linsi
@ 2020-04-30 13:45 ` Lance Richardson
  2020-04-30 18:29   ` Ajit Khaparde
  2020-04-30 23:55 ` Ferruh Yigit
  1 sibling, 1 reply; 14+ messages in thread
From: Lance Richardson @ 2020-04-30 13:45 UTC (permalink / raw)
  To: Yuan Linsi; +Cc: Ajit Kumar Khaparde, Somnath Kotur, dev

On Thu, Apr 30, 2020 at 9:37 AM Yuan Linsi <yuanlinsi01@baidu.com> wrote:
>
> From: yuanlinsi01 <yuanlinsi01@baidu.com>
>
> We see a stack smashing as a result of defensive code missing. Once the
> nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
> zero after doing a floor align, and we can not exit the following
> receiving packets loop. And the buffers will be overwrite, then the
> stack frame was ruined.
>
> Fix the problem by adding defensive code, once the nb_pkts is zero, just
> directly return with no packets.
>
> Fixes: bc4a000f2 ("net/bnxt: implement SSE vector mode")
> Cc: stable@dpdk.org
>
> Signed-off-by: yuanlinsi01 <yuanlinsi01@baidu.com>
> Signed-off-by: rongdongsheng <rongdongsheng@baidu.com>
> ---
>  drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> index d0e7910e7..8f73add9b 100644
> --- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> +++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
> @@ -233,8 +233,13 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
>         /* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
>         nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
>
> -       /* Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP */
> +       /*
> +        * Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP.
> +        * nb_pkts < RTE_BNXT_DESCS_PER_LOOP, just return no packet
> +        */
>         nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_BNXT_DESCS_PER_LOOP);
> +       if (!nb_pkts)
> +               return 0;
>
>         /* Handle RX burst request */
>         while (1) {
> --
> 2.11.0
>
Thanks for the fix!

Acked-by: Lance Richardson <lance.richardson@broadcom.com>

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

* [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
@ 2020-04-30 13:37 Yuan Linsi
  2020-04-30 13:45 ` Lance Richardson
  2020-04-30 23:55 ` Ferruh Yigit
  0 siblings, 2 replies; 14+ messages in thread
From: Yuan Linsi @ 2020-04-30 13:37 UTC (permalink / raw)
  To: ajit.khaparde, somnath.kotur, lance.richardson; +Cc: dev

From: yuanlinsi01 <yuanlinsi01@baidu.com>

We see a stack smashing as a result of defensive code missing. Once the
nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
zero after doing a floor align, and we can not exit the following
receiving packets loop. And the buffers will be overwrite, then the
stack frame was ruined.

Fix the problem by adding defensive code, once the nb_pkts is zero, just
directly return with no packets.

Fixes: bc4a000f2 ("net/bnxt: implement SSE vector mode")
Cc: stable@dpdk.org

Signed-off-by: yuanlinsi01 <yuanlinsi01@baidu.com>
Signed-off-by: rongdongsheng <rongdongsheng@baidu.com>
---
 drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
index d0e7910e7..8f73add9b 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
@@ -233,8 +233,13 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	/* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
 	nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
 
-	/* Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP */
+	/*
+	 * Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP.
+	 * nb_pkts < RTE_BNXT_DESCS_PER_LOOP, just return no packet
+	 */
 	nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_BNXT_DESCS_PER_LOOP);
+	if (!nb_pkts)
+		return 0;
 
 	/* Handle RX burst request */
 	while (1) {
-- 
2.11.0


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

* [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
@ 2020-04-30 10:08 yuanlinsi01
  0 siblings, 0 replies; 14+ messages in thread
From: yuanlinsi01 @ 2020-04-30 10:08 UTC (permalink / raw)
  To: ajit.khaparde, somnath.kotur; +Cc: dev

We see a stack smashing as a result of defensive code missing. Once the
nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
zero after doing a floor align, and we can not exit the following
receiving packets loop. And the buffers will be overwrite, then the
stack frame was ruined.

Fix the problem by adding defensive code, once the nb_pkts is zero, just
directly return with no packets.

__GI___backtrace (array=0x7fcec7ac3f00, size=256) at ../sysdeps/x86_64/backtrace.c:103
catch_segfault () from /lib64/libSegFault.so
<signal handler called>
__GI___backtrace (array=array@entry=0x7fcec7ac62e0, size=size@entry=64) at ../sysdeps/x86_64/backtrace.c:103
backtrace_and_maps (do_abort=do_abort@entry=2, written=<optimized out>, fd=fd@entry=2) at ../sysdeps/unix/sysv/linux/libc_fatal.c:47
__libc_message (do_abort=do_abort@entry=2, fmt=fmt@entry=0x7fced6091c60 "*** %s ***: %s terminated\n") at ../sysdeps/posix/libc_fatal.c:172
__GI___fortify_fail (msg=msg@entry=0x7fced6091c48 "stack smashing detected") at fortify_fail.c:31
__stack_chk_fail () at stack_chk_fail.c:28
bnxt_recv_pkts_vec (rx_queue=0x14c571f00, rx_pkts=0x7fcec7ac6f28, nb_pkts=0)
rte_eth_rx_burst (port_id=1, queue_id=3, rx_pkts=0x7fcec7ac6f28, nb_pkts=1)

Signed-off-by: yuanlinsi01 <yuanlinsi01@baidu.com>
Signed-off-by: rongdongsheng <rongdongsheng@baidu.com>
---
 drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
index d0e7910e7..c4adccdbc 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
@@ -233,8 +233,13 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	/* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
 	nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
 
-	/* Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP */
+	/*
+	 * Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP
+	 * nb_pkts < RTE_BNXT_DESCS_PER_LOOP, just return no packet
+	 */
 	nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_BNXT_DESCS_PER_LOOP);
+	if (!nb_pkts)
+		return 0;
 
 	/* Handle RX burst request */
 	while (1) {
-- 
2.11.0


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

* [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing
@ 2020-04-30 10:07 yuanlinsi01
  0 siblings, 0 replies; 14+ messages in thread
From: yuanlinsi01 @ 2020-04-30 10:07 UTC (permalink / raw)
  To: ajit.khaparde, somnath.kotur; +Cc: dev

We see a stack smashing as a result of defensive code missing. Once the
nb_pkts is less than RTE_BNXT_DESCS_PER_LOOP, it will be modified to
zero after doing a floor align, and we can not exit the following
receiving packets loop. And the buffers will be overwrite, then the
stack frame was ruined.

Fix the problem by adding defensive code, once the nb_pkts is zero, just
directly return with no packets.

__GI___backtrace (array=0x7fcec7ac3f00, size=256) at ../sysdeps/x86_64/backtrace.c:103
catch_segfault () from /lib64/libSegFault.so
<signal handler called>
__GI___backtrace (array=array@entry=0x7fcec7ac62e0, size=size@entry=64) at ../sysdeps/x86_64/backtrace.c:103
backtrace_and_maps (do_abort=do_abort@entry=2, written=<optimized out>, fd=fd@entry=2) at ../sysdeps/unix/sysv/linux/libc_fatal.c:47
__libc_message (do_abort=do_abort@entry=2, fmt=fmt@entry=0x7fced6091c60 "*** %s ***: %s terminated\n") at ../sysdeps/posix/libc_fatal.c:172
__GI___fortify_fail (msg=msg@entry=0x7fced6091c48 "stack smashing detected") at fortify_fail.c:31
__stack_chk_fail () at stack_chk_fail.c:28
bnxt_recv_pkts_vec (rx_queue=0x14c571f00, rx_pkts=0x7fcec7ac6f28, nb_pkts=0)
rte_eth_rx_burst (port_id=1, queue_id=3, rx_pkts=0x7fcec7ac6f28, nb_pkts=1)

Signed-off-by: yuanlinsi01 <yuanlinsi01@baidu.com>
Signed-off-by: rongdongsheng <rongdongsheng@baidu.com>
---
 drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
index d0e7910e7..c4adccdbc 100644
--- a/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
+++ b/drivers/net/bnxt/bnxt_rxtx_vec_sse.c
@@ -233,8 +233,13 @@ bnxt_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	/* Return no more than RTE_BNXT_MAX_RX_BURST per call. */
 	nb_pkts = RTE_MIN(nb_pkts, RTE_BNXT_MAX_RX_BURST);
 
-	/* Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP */
+	/*
+	 * Make nb_pkts an integer multiple of RTE_BNXT_DESCS_PER_LOOP
+	 * nb_pkts < RTE_BNXT_DESCS_PER_LOOP, just return no packet
+	 */
 	nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_BNXT_DESCS_PER_LOOP);
+	if (!nb_pkts)
+		return 0;
 
 	/* Handle RX burst request */
 	while (1) {
-- 
2.11.0


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

end of thread, other threads:[~2020-05-06  5:26 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-30 12:05 [dpdk-dev] [PATCH] net/bnxt: fix a possible stack smashing yuanlinsi01
2020-04-30 12:55 ` Somnath Kotur
2020-04-30 13:33   ` [dpdk-dev] 答复: " Yuan,Linsi
  -- strict thread matches above, loose matches on Subject: below --
2020-05-06  3:28 [dpdk-dev] " Yuan Linsi
2020-05-06  5:26 ` Ajit Khaparde
2020-05-06  3:18 Yuan Linsi
2020-04-30 13:37 Yuan Linsi
2020-04-30 13:45 ` Lance Richardson
2020-04-30 18:29   ` Ajit Khaparde
2020-04-30 23:55 ` Ferruh Yigit
2020-05-05  3:42   ` Ajit Khaparde
2020-05-06  3:18     ` [dpdk-dev] 答复: " Yuan,Linsi
2020-04-30 10:08 [dpdk-dev] " yuanlinsi01
2020-04-30 10:07 yuanlinsi01

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git