net/mlx5: fix destroying empty matchers list

Message ID 20220223093205.4240-2-adham@nvidia.com (mailing list archive)
State Accepted, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5: fix destroying empty matchers list |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Adham Masarwah Feb. 23, 2022, 9:32 a.m. UTC
  The table remove callback function is trying to destroy the
matchers list associated with table entries without checking
if the list is valid, which causes nullptr dereference.
Fixed by validating the matchers list before destroying it.

Issue can be reproduced with testpmd on Windows, when you run:
port close all

Fixes: 1872635570be ("net/mlx5: make matcher list thread safe")
Cc: stable@dpdk.org

Signed-off-by: Adham Masarwah <adham@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Tal Shnaiderman Feb. 23, 2022, 10:39 a.m. UTC | #1
> Subject: [PATCH] net/mlx5: fix destroying empty matchers list
> 
> The table remove callback function is trying to destroy the matchers list
> associated with table entries without checking if the list is valid, which causes
> nullptr dereference.
> Fixed by validating the matchers list before destroying it.
> 
> Issue can be reproduced with testpmd on Windows, when you run:
> port close all
> 
> Fixes: 1872635570be ("net/mlx5: make matcher list thread safe")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Adham Masarwah <adham@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> ---
>  drivers/net/mlx5/mlx5_flow_dv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index ef9c66eddf..baef252a7a 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -10503,7 +10503,8 @@ flow_dv_tbl_remove_cb(void *tool_ctx, struct
> mlx5_list_entry *entry)
>  			tbl_data->tunnel->tunnel_id : 0,
>  			tbl_data->group_id);
>  	}
> -	mlx5_list_destroy(tbl_data->matchers);
> +	if (tbl_data->matchers)
> +		mlx5_list_destroy(tbl_data->matchers);
>  	mlx5_ipool_free(sh->ipool[MLX5_IPOOL_JUMP], tbl_data->idx);  }
> 
> --
> 2.16.1.windows.4

Acked-by: Tal Shnaiderman <talshn@nvidia.com>
  
Idan Hackmon Feb. 27, 2022, 12:38 p.m. UTC | #2
> -----Original Message-----
> From: Adham Masarwah <adham@nvidia.com>
> Sent: Sunday, February 27, 2022 2:33 PM
> To: Idan Hackmon <idanhac@nvidia.com>
> Subject: RE: [PATCH] net/mlx5: fix destroying empty matchers list
> 
> > Subject: [PATCH] net/mlx5: fix destroying empty matchers list
> >
> > The table remove callback function is trying to destroy the matchers
> > list associated with table entries without checking if the list is
> > valid, which causes nullptr dereference.
> > Fixed by validating the matchers list before destroying it.
> >
> > Issue can be reproduced with testpmd on Windows, when you run:
> > port close all
> >
> > Fixes: 1872635570be ("net/mlx5: make matcher list thread safe")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Adham Masarwah <adham@nvidia.com>
> > Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> > ---
> >  drivers/net/mlx5/mlx5_flow_dv.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> > b/drivers/net/mlx5/mlx5_flow_dv.c index ef9c66eddf..baef252a7a 100644
> > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > @@ -10503,7 +10503,8 @@ flow_dv_tbl_remove_cb(void *tool_ctx, struct
> > mlx5_list_entry *entry)
> >  			tbl_data->tunnel->tunnel_id : 0,
> >  			tbl_data->group_id);
> >  	}
> > -	mlx5_list_destroy(tbl_data->matchers);
> > +	if (tbl_data->matchers)
> > +		mlx5_list_destroy(tbl_data->matchers);
> >  	mlx5_ipool_free(sh->ipool[MLX5_IPOOL_JUMP], tbl_data->idx);  }
> >
> > --
> > 2.16.1.windows.4
> 
> Acked-by: Tal Shnaiderman <talshn@nvidia.com>
Tested-by: Idan Hackmon <idanhac@nvidia.com>
  
Raslan Darawsheh March 2, 2022, 10:20 a.m. UTC | #3
Hi,

> -----Original Message-----
> From: Adham Masarwah <adham@nvidia.com>
> Sent: Wednesday, February 23, 2022 11:32 AM
> To: dev@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
> Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Tal Shnaiderman <talshn@nvidia.com>; Tamer
> Hleihel <tamerh@nvidia.com>; erfrimod@microsoft.com; stable@dpdk.org
> Subject: [PATCH] net/mlx5: fix destroying empty matchers list
> 
> The table remove callback function is trying to destroy the
> matchers list associated with table entries without checking
> if the list is valid, which causes nullptr dereference.
> Fixed by validating the matchers list before destroying it.
> 
> Issue can be reproduced with testpmd on Windows, when you run:
> port close all
> 
> Fixes: 1872635570be ("net/mlx5: make matcher list thread safe")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Adham Masarwah <adham@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh
  

Patch

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index ef9c66eddf..baef252a7a 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -10503,7 +10503,8 @@  flow_dv_tbl_remove_cb(void *tool_ctx, struct mlx5_list_entry *entry)
 			tbl_data->tunnel->tunnel_id : 0,
 			tbl_data->group_id);
 	}
-	mlx5_list_destroy(tbl_data->matchers);
+	if (tbl_data->matchers)
+		mlx5_list_destroy(tbl_data->matchers);
 	mlx5_ipool_free(sh->ipool[MLX5_IPOOL_JUMP], tbl_data->idx);
 }