RE: [PATCH net v3] net/mana: Fix auxiliary device double-delete race

[Long Li]

> From: Shiraz Saleem <shirazsaleem@xxxxxxxxxxxxx>
> 
> Make remove_adev() safe to call concurrently from the service reset and PCI
> eject paths by using xchg() to atomically claim the adev pointer. This prevents
> double auxiliary_device_delete/uninit when hv_eject_device_work races with
> the service reset workqueue.
> 
> Fixes: 505cc26bcae0 ("net: mana: Add support for auxiliary device servicing
> events")
> Signed-off-by: Shiraz Saleem <shirazsaleem@xxxxxxxxxxxxx>
> Signed-off-by: Konstantin Taranov <kotaranov@xxxxxxxxxxxxx>

Reviewed-by: Long Li <longli@xxxxxxxxxxxxx>


> ---
> v3: Separate the xchg() call from the variable declaration in remove_adev() to
> avoid calling functions with side effects as variable initializers
> v2: rebased on the latest net
>  drivers/net/ethernet/microsoft/mana/mana_en.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
> b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index a654b3699..dd4f4215a 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -3465,14 +3465,19 @@ static void adev_release(struct device *dev)
> 
>  static void remove_adev(struct gdma_dev *gd)  {
> -	struct auxiliary_device *adev = gd->adev;
> -	int id = adev->id;
> +	struct auxiliary_device *adev;
> +	int id;
> +
> +	adev = xchg(&gd->adev, NULL);
> +	if (!adev)
> +		return;
> +
> +	id = adev->id;
> 
>  	auxiliary_device_delete(adev);
>  	auxiliary_device_uninit(adev);
> 
>  	mana_adev_idx_free(id);
> -	gd->adev = NULL;
>  }
> 
>  static int add_adev(struct gdma_dev *gd, const char *name) @@ -3538,7
> +3543,7 @@ static void mana_rdma_service_handle(struct work_struct *work)
> 
>  	switch (serv_work->event) {
>  	case GDMA_SERVICE_TYPE_RDMA_SUSPEND:
> -		if (!gd->adev || gd->is_suspended)
> +		if (gd->is_suspended)
>  			break;
> 
>  		remove_adev(gd);
> @@ -3753,8 +3758,7 @@ void mana_remove(struct gdma_dev *gd, bool
> suspending)
>  	cancel_delayed_work_sync(&ac->gf_stats_work);
> 
>  	/* adev currently doesn't support suspending, always remove it */
> -	if (gd->adev)
> -		remove_adev(gd);
> +	remove_adev(gd);
> 
>  	for (i = 0; i < ac->num_ports; i++) {
>  		ndev = ac->ports[i];
> @@ -3843,8 +3847,7 @@ void mana_rdma_remove(struct gdma_dev *gd)
>  	if (gc->service_wq)
>  		flush_workqueue(gc->service_wq);
> 
> -	if (gd->adev)
> -		remove_adev(gd);
> +	remove_adev(gd);
> 
>  	mana_gd_deregister_device(gd);
>  }
> --
> 2.43.0