On Mon, Mar 20, 2023 at 5:35 PM Eli Cohen <elic@xxxxxxxxxx> wrote:
>
>
> On 20/03/2023 11:28, Jason Wang wrote:
> > On Mon, Mar 20, 2023 at 5:07 PM Eli Cohen <elic@xxxxxxxxxx> wrote:
> >> On 17/03/2023 5:32, Jason Wang wrote:
> >>> On Sun, Mar 12, 2023 at 4:41 PM Eli Cohen <elic@xxxxxxxxxx> wrote:
> >>>> When deleting the vdpa device, the debugfs files need to be removed so
> >>>> need to remove debugfs after the device has been unregistered.
> >>>>
> >>>> This fixes null pointer dereference when someone deletes the device
> >>>> after debugfs has been populated.
> >>>>
> >>>> Fixes: 294221004322 ("vdpa/mlx5: Add debugfs subtree")
> >>>> Signed-off-by: Eli Cohen <elic@xxxxxxxxxx>
> >>>> ---
> >>>> drivers/vdpa/mlx5/net/mlx5_vnet.c | 4 ++--
> >>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> >>>> index 3858ba1e8975..3f6149f2ffd4 100644
> >>>> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> >>>> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> >>>> @@ -3322,8 +3322,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;
> >>>> if (ndev->nb_registered) {
> >>>> ndev->nb_registered = false;
> >>>> mlx5_notifier_unregister(mvdev->mdev, &ndev->nb);
> >>>> @@ -3332,6 +3330,8 @@ static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev *v_mdev, struct vdpa_device *
> >>>> mvdev->wq = NULL;
> >>>> destroy_workqueue(wq);
> >>>> _vdpa_unregister_device(dev);
> >>> What if the user tries to access debugfs after _vdpa_unregister_device()?
> >> I don't see an issue with that. During the process of deleting a device,
> >> the resources are destroyed and their corresponding debugfs files are
> >> also destroyed just prior to destroying the resource.
> > For example, rx_flow_table_show() requires the structure mlx5_vdpa_net
> > alive, but it seems the structure has been freed after
> > _vdpa_unregister_device().
> But in this case, rx_flow_table_show() gets called first, so the file
> will be removed from the debugfs before
>
> mlx5_vdpa_net gets released.
Just to make sure we are at the same page. I meant:
[CPU 0] _vdpa_unregister_device(dev);
[CPU 1] rx_flow_table_show()
[CPU 0] mlx5_vdpa_remove_debugfs()
So when CPU 1 tries to access the flow table, the ndev has been
released by CPU0 in _vdpa_unregister_device()?
Thanks
>
> >
> > If a user tries to access that file after _vdpa_unregister_device()
> > but before mlx5_vdpa_remove_debugfs(), will we end up with
> > use-after-free?
> >
> > Thanks
> >
> >
> >>> Thanks
> >>>
> >>>> + mlx5_vdpa_remove_debugfs(ndev->debugfs);
> >>>> + ndev->debugfs = NULL;
> >>>> mgtdev->ndev = NULL;
> >>>> }
> >>>>
> >>>> --
> >>>> 2.38.1
> >>>>
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
