Dragos Tatulea
2023-Aug-29 17:40 UTC
[PATCH] vdpa/mlx5: Fix double release of debugfs entry
The error path in setup_driver deletes the debugfs entry but doesn't
clear the pointer. During .dev_del the invalid pointer will be released
again causing a crash.
This patch fixes the issue by always clearing the debugfs entry in
mlx5_vdpa_remove_debugfs. Also, stop removing the debugfs entry in
.dev_del op: the debugfs entry is already handled within the
setup_driver/teardown_driver scope.
Fixes: f0417e72add5 ("vdpa/mlx5: Add and remove debugfs in setup/teardown
driver")
Signed-off-by: Dragos Tatulea <dtatulea at nvidia.com>
Reviewed-by: Gal Pressman <gal at nvidia.com>
---
drivers/vdpa/mlx5/net/debug.c | 5 +++--
drivers/vdpa/mlx5/net/mlx5_vnet.c | 7 ++-----
drivers/vdpa/mlx5/net/mlx5_vnet.h | 2 +-
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/debug.c b/drivers/vdpa/mlx5/net/debug.c
index 60d6ac68cdc4..9c85162c19fc 100644
--- a/drivers/vdpa/mlx5/net/debug.c
+++ b/drivers/vdpa/mlx5/net/debug.c
@@ -146,7 +146,8 @@ void mlx5_vdpa_add_debugfs(struct mlx5_vdpa_net *ndev)
ndev->rx_dent = debugfs_create_dir("rx", ndev->debugfs);
}
-void mlx5_vdpa_remove_debugfs(struct dentry *dbg)
+void mlx5_vdpa_remove_debugfs(struct mlx5_vdpa_net *ndev)
{
- debugfs_remove_recursive(dbg);
+ debugfs_remove_recursive(ndev->debugfs);
+ ndev->debugfs = NULL;
}
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 37be945a0230..f91c938b4be1 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2713,7 +2713,7 @@ static int setup_driver(struct mlx5_vdpa_dev *mvdev)
err_rqt:
teardown_virtqueues(ndev);
err_setup:
- mlx5_vdpa_remove_debugfs(ndev->debugfs);
+ mlx5_vdpa_remove_debugfs(ndev);
out:
return err;
}
@@ -2727,8 +2727,7 @@ static void teardown_driver(struct mlx5_vdpa_net *ndev)
if (!ndev->setup)
return;
- mlx5_vdpa_remove_debugfs(ndev->debugfs);
- ndev->debugfs = NULL;
+ mlx5_vdpa_remove_debugfs(ndev);
teardown_steering(ndev);
destroy_tir(ndev);
destroy_rqt(ndev);
@@ -3489,8 +3488,6 @@ static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev
*v_mdev, struct vdpa_device *
struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
struct workqueue_struct *wq;
- mlx5_vdpa_remove_debugfs(ndev->debugfs);
- ndev->debugfs = NULL;
unregister_link_notifier(ndev);
_vdpa_unregister_device(dev);
wq = mvdev->wq;
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.h
b/drivers/vdpa/mlx5/net/mlx5_vnet.h
index 36c44d9fdd16..60cdbc903037 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.h
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.h
@@ -88,7 +88,7 @@ struct macvlan_node {
};
void mlx5_vdpa_add_debugfs(struct mlx5_vdpa_net *ndev);
-void mlx5_vdpa_remove_debugfs(struct dentry *dbg);
+void mlx5_vdpa_remove_debugfs(struct mlx5_vdpa_net *ndev);
void mlx5_vdpa_add_rx_flow_table(struct mlx5_vdpa_net *ndev);
void mlx5_vdpa_remove_rx_flow_table(struct mlx5_vdpa_net *ndev);
void mlx5_vdpa_add_tirn(struct mlx5_vdpa_net *ndev);
--
2.41.0
Dragos Tatulea
2023-Sep-14 09:40 UTC
[PATCH] vdpa/mlx5: Fix double release of debugfs entry
On Tue, 2023-08-29 at 20:40 +0300, Dragos Tatulea wrote:> The error path in setup_driver deletes the debugfs entry but doesn't > clear the pointer. During .dev_del the invalid pointer will be released > again causing a crash. > > This patch fixes the issue by always clearing the debugfs entry in > mlx5_vdpa_remove_debugfs. Also, stop removing the debugfs entry in > .dev_del op: the debugfs entry is already handled within the > setup_driver/teardown_driver scope. > > Fixes: f0417e72add5 ("vdpa/mlx5: Add and remove debugfs in setup/teardown > driver") > Signed-off-by: Dragos Tatulea <dtatulea at nvidia.com> > Reviewed-by: Gal Pressman <gal at nvidia.com>Gentle ping. Are there any comments on this patch? Thanks, Dragos> --- > ?drivers/vdpa/mlx5/net/debug.c???? | 5 +++-- > ?drivers/vdpa/mlx5/net/mlx5_vnet.c | 7 ++----- > ?drivers/vdpa/mlx5/net/mlx5_vnet.h | 2 +- > ?3 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/drivers/vdpa/mlx5/net/debug.c b/drivers/vdpa/mlx5/net/debug.c > index 60d6ac68cdc4..9c85162c19fc 100644 > --- a/drivers/vdpa/mlx5/net/debug.c > +++ b/drivers/vdpa/mlx5/net/debug.c > @@ -146,7 +146,8 @@ void mlx5_vdpa_add_debugfs(struct mlx5_vdpa_net *ndev) > ????????????????ndev->rx_dent = debugfs_create_dir("rx", ndev->debugfs); > ?} > ? > -void mlx5_vdpa_remove_debugfs(struct dentry *dbg) > +void mlx5_vdpa_remove_debugfs(struct mlx5_vdpa_net *ndev) > ?{ > -???????debugfs_remove_recursive(dbg); > +???????debugfs_remove_recursive(ndev->debugfs); > +???????ndev->debugfs = NULL; > ?} > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c > b/drivers/vdpa/mlx5/net/mlx5_vnet.c > index 37be945a0230..f91c938b4be1 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > @@ -2713,7 +2713,7 @@ static int setup_driver(struct mlx5_vdpa_dev *mvdev) > ?err_rqt: > ????????teardown_virtqueues(ndev); > ?err_setup: > -???????mlx5_vdpa_remove_debugfs(ndev->debugfs); > +???????mlx5_vdpa_remove_debugfs(ndev); > ?out: > ????????return err; > ?} > @@ -2727,8 +2727,7 @@ static void teardown_driver(struct mlx5_vdpa_net *ndev) > ????????if (!ndev->setup) > ????????????????return; > ? > -???????mlx5_vdpa_remove_debugfs(ndev->debugfs); > -???????ndev->debugfs = NULL; > +???????mlx5_vdpa_remove_debugfs(ndev); > ????????teardown_steering(ndev); > ????????destroy_tir(ndev); > ????????destroy_rqt(ndev); > @@ -3489,8 +3488,6 @@ static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev > *v_mdev, struct vdpa_device * > ????????struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev); > ????????struct workqueue_struct *wq; > ? > -???????mlx5_vdpa_remove_debugfs(ndev->debugfs); > -???????ndev->debugfs = NULL; > ????????unregister_link_notifier(ndev); > ????????_vdpa_unregister_device(dev); > ????????wq = mvdev->wq; > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.h > b/drivers/vdpa/mlx5/net/mlx5_vnet.h > index 36c44d9fdd16..60cdbc903037 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.h > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.h > @@ -88,7 +88,7 @@ struct macvlan_node { > ?}; > ? > ?void mlx5_vdpa_add_debugfs(struct mlx5_vdpa_net *ndev); > -void mlx5_vdpa_remove_debugfs(struct dentry *dbg); > +void mlx5_vdpa_remove_debugfs(struct mlx5_vdpa_net *ndev); > ?void mlx5_vdpa_add_rx_flow_table(struct mlx5_vdpa_net *ndev); > ?void mlx5_vdpa_remove_rx_flow_table(struct mlx5_vdpa_net *ndev); > ?void mlx5_vdpa_add_tirn(struct mlx5_vdpa_net *ndev);
On Wed, Aug 30, 2023 at 1:40?AM Dragos Tatulea <dtatulea at nvidia.com> wrote:> > The error path in setup_driver deletes the debugfs entry but doesn't > clear the pointer. During .dev_del the invalid pointer will be released > again causing a crash. > > This patch fixes the issue by always clearing the debugfs entry in > mlx5_vdpa_remove_debugfs. Also, stop removing the debugfs entry in > .dev_del op: the debugfs entry is already handled within the > setup_driver/teardown_driver scope. > > Fixes: f0417e72add5 ("vdpa/mlx5: Add and remove debugfs in setup/teardown driver") > Signed-off-by: Dragos Tatulea <dtatulea at nvidia.com> > Reviewed-by: Gal Pressman <gal at nvidia.com>Acked-by: Jason Wang <jasowang at redhat.com> Thanks> --- > drivers/vdpa/mlx5/net/debug.c | 5 +++-- > drivers/vdpa/mlx5/net/mlx5_vnet.c | 7 ++----- > drivers/vdpa/mlx5/net/mlx5_vnet.h | 2 +- > 3 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/drivers/vdpa/mlx5/net/debug.c b/drivers/vdpa/mlx5/net/debug.c > index 60d6ac68cdc4..9c85162c19fc 100644 > --- a/drivers/vdpa/mlx5/net/debug.c > +++ b/drivers/vdpa/mlx5/net/debug.c > @@ -146,7 +146,8 @@ void mlx5_vdpa_add_debugfs(struct mlx5_vdpa_net *ndev) > ndev->rx_dent = debugfs_create_dir("rx", ndev->debugfs); > } > > -void mlx5_vdpa_remove_debugfs(struct dentry *dbg) > +void mlx5_vdpa_remove_debugfs(struct mlx5_vdpa_net *ndev) > { > - debugfs_remove_recursive(dbg); > + debugfs_remove_recursive(ndev->debugfs); > + ndev->debugfs = NULL; > } > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > index 37be945a0230..f91c938b4be1 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > @@ -2713,7 +2713,7 @@ static int setup_driver(struct mlx5_vdpa_dev *mvdev) > err_rqt: > teardown_virtqueues(ndev); > err_setup: > - mlx5_vdpa_remove_debugfs(ndev->debugfs); > + mlx5_vdpa_remove_debugfs(ndev); > out: > return err; > } > @@ -2727,8 +2727,7 @@ static void teardown_driver(struct mlx5_vdpa_net *ndev) > if (!ndev->setup) > return; > > - mlx5_vdpa_remove_debugfs(ndev->debugfs); > - ndev->debugfs = NULL; > + mlx5_vdpa_remove_debugfs(ndev); > teardown_steering(ndev); > destroy_tir(ndev); > destroy_rqt(ndev); > @@ -3489,8 +3488,6 @@ static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev *v_mdev, struct vdpa_device * > struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev); > struct workqueue_struct *wq; > > - mlx5_vdpa_remove_debugfs(ndev->debugfs); > - ndev->debugfs = NULL; > unregister_link_notifier(ndev); > _vdpa_unregister_device(dev); > wq = mvdev->wq; > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.h b/drivers/vdpa/mlx5/net/mlx5_vnet.h > index 36c44d9fdd16..60cdbc903037 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.h > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.h > @@ -88,7 +88,7 @@ struct macvlan_node { > }; > > void mlx5_vdpa_add_debugfs(struct mlx5_vdpa_net *ndev); > -void mlx5_vdpa_remove_debugfs(struct dentry *dbg); > +void mlx5_vdpa_remove_debugfs(struct mlx5_vdpa_net *ndev); > void mlx5_vdpa_add_rx_flow_table(struct mlx5_vdpa_net *ndev); > void mlx5_vdpa_remove_rx_flow_table(struct mlx5_vdpa_net *ndev); > void mlx5_vdpa_add_tirn(struct mlx5_vdpa_net *ndev); > -- > 2.41.0 >