| Message ID | 20240604100552.25201-1-amishin@t-argos.ru (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 229bedbf62b13af5aba6525ad10b62ad38d9ccb5 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net/mlx5: Fix tainted pointer delete is case of flow rules creation fail | expand |
On 6/4/2024 3:05 AM, Aleksandr Mishin wrote: > In case of flow rule creation fail in mlx5_lag_create_port_sel_table(), > instead of previously created rules, the tainted pointer is deleted > deveral times. > Fix this bug by using correct flow rules pointers. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Fixes: 352899f384d4 ("net/mlx5: Lag, use buckets in hash mode") > Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru> > --- > drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c > index c16b462ddedf..ab2717012b79 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c > @@ -88,9 +88,13 @@ static int mlx5_lag_create_port_sel_table(struct mlx5_lag *ldev, > &dest, 1); > if (IS_ERR(lag_definer->rules[idx])) { > err = PTR_ERR(lag_definer->rules[idx]); > - while (i--) > - while (j--) > + do { > + while (j--) { > + idx = i * ldev->buckets + j; > mlx5_del_flow_rules(lag_definer->rules[idx]); > + } > + j = ldev->buckets; > + } while (i--); So, before the code was: while (i--) while (j--) mlx5_del_flow_rules(lag_definer->rules[idx]); That just calls mlx5_del_flow_rules a bunch of times but keeps using the wrong index, which is obviously wrong. The new fix is to calculate the index properly (hence the switch to a do { } while (0) loop, so that we properly delete all of the older rules rather than calling mlx5_del_flow_rules multiple times on the wrong index. Makes sense. Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > goto destroy_fg; > } > }
On 04/06/2024 13:05, Aleksandr Mishin wrote: > In case of flow rule creation fail in mlx5_lag_create_port_sel_table(), > instead of previously created rules, the tainted pointer is deleted > deveral times. > Fix this bug by using correct flow rules pointers. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Fixes: 352899f384d4 ("net/mlx5: Lag, use buckets in hash mode") > Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru> > --- > drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c > index c16b462ddedf..ab2717012b79 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c > @@ -88,9 +88,13 @@ static int mlx5_lag_create_port_sel_table(struct mlx5_lag *ldev, > &dest, 1); > if (IS_ERR(lag_definer->rules[idx])) { > err = PTR_ERR(lag_definer->rules[idx]); > - while (i--) > - while (j--) > + do { > + while (j--) { > + idx = i * ldev->buckets + j; > mlx5_del_flow_rules(lag_definer->rules[idx]); > + } > + j = ldev->buckets; > + } while (i--); > goto destroy_fg; > } > } Reviewed-by: Tariq Toukan <tariqt@nvidia.com> Thanks.
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 4 Jun 2024 13:05:52 +0300 you wrote: > In case of flow rule creation fail in mlx5_lag_create_port_sel_table(), > instead of previously created rules, the tainted pointer is deleted > deveral times. > Fix this bug by using correct flow rules pointers. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > [...] Here is the summary with links: - [net] net/mlx5: Fix tainted pointer delete is case of flow rules creation fail https://git.kernel.org/netdev/net/c/229bedbf62b1 You are awesome, thank you!
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c index c16b462ddedf..ab2717012b79 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c @@ -88,9 +88,13 @@ static int mlx5_lag_create_port_sel_table(struct mlx5_lag *ldev, &dest, 1); if (IS_ERR(lag_definer->rules[idx])) { err = PTR_ERR(lag_definer->rules[idx]); - while (i--) - while (j--) + do { + while (j--) { + idx = i * ldev->buckets + j; mlx5_del_flow_rules(lag_definer->rules[idx]); + } + j = ldev->buckets; + } while (i--); goto destroy_fg; } }
In case of flow rule creation fail in mlx5_lag_create_port_sel_table(), instead of previously created rules, the tainted pointer is deleted deveral times. Fix this bug by using correct flow rules pointers. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 352899f384d4 ("net/mlx5: Lag, use buckets in hash mode") Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru> --- drivers/net/ethernet/mellanox/mlx5/core/lag/port_sel.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)