net/bnxt: fix a possible stack smashing

Message ID 1588253872-19024-1-git-send-email-yuanlinsi01@baidu.com (mailing list archive)
State Accepted, archived
Delegated to: Ajit Khaparde
Headers
Series net/bnxt: fix a possible stack smashing |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-testing success Testing PASS

Commit Message

Yuan Linsi April 30, 2020, 1:37 p.m. UTC
  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(-)
  

Comments

Lance Richardson April 30, 2020, 1:45 p.m. UTC | #1
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>
  
Ajit Khaparde April 30, 2020, 6:29 p.m. UTC | #2
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
  
Ferruh Yigit April 30, 2020, 11:55 p.m. UTC | #3
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'?
  
Ajit Khaparde May 5, 2020, 3:42 a.m. UTC | #4
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
  
Yuan Linsi May 6, 2020, 3:18 a.m. UTC | #5
Ok, I'll modify it per you suggestion

Thanks,
Yuan Linsi
  

Patch

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