RE: [PATCH iwl-net v2 1/5] ice: check the auxiliary device pointer under adev_mutex

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Petr Oros <poros@xxxxxxxxxx>
> Sent: Friday, October 23, 2026 9:03 AM
> To: netdev@xxxxxxxxxxxxxxx
> Cc: Oros, Petr <poros@xxxxxxxxxx>; Nguyen, Anthony L
> <anthony.l.nguyen@xxxxxxxxx>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>; Andrew Lunn <andrew+netdev@xxxxxxx>;
> David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>;
> Clark Williams <clrkwllms@xxxxxxxxxx>; Steven Rostedt
> <rostedt@xxxxxxxxxxx>; Ertman, David M <david.m.ertman@xxxxxxxxx>;
> Mateusz Palczewski <mateusz.palczewski@xxxxxxxxx>; Leon Romanovsky
> <leon@xxxxxxxxxx>; Rafal Rogalski <rafalx.rogalski@xxxxxxxxx>; Shiraz
> Saleem <shiraz.saleem@xxxxxxxxx>; Loktionov, Aleksandr
> <aleksandr.loktionov@xxxxxxxxx>; intel-wired-lan@xxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; linux-rt-devel@xxxxxxxxxxxxxxx
> Subject: [PATCH iwl-net v2 1/5] ice: check the auxiliary device pointer
> under adev_mutex
> 
> ice_set_channels() and the mqprio path of ice_setup_tc() test
> cdev->adev before taking pf->adev_mutex and dereference it afterwards.
> ice_unplug_aux_dev() clears the pointer under the mutex from the service
> task, from ice_prepare_for_reset() and from the devlink RoCE and iWARP
> parameter handlers, none of which hold the RTNL lock, so the pointer can
> go away between the test and the device_lock() call.
> 
> Take pf->adev_mutex first and test the pointer under it. Without a
> device the mutex is released again and the rebuild runs unlocked as
> before.
> 
> Fixes: a6a0974aae42 ("ice: Prevent set_channel from changing queues
> while RDMA active")
> Fixes: 4b31fd4d77ff ("ice: Fix RDMA VSI removal during queue rebuild")
> Signed-off-by: Petr Oros <poros@xxxxxxxxxx>
> ---
> v2:
> - new patch, the pointer test raced with ice_unplug_aux_dev()
> ---
>  drivers/net/ethernet/intel/ice/ice_ethtool.c | 22 +++++++++++++-------
>  drivers/net/ethernet/intel/ice/ice_main.c    | 22 +++++++++++++-------
>  2 files changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index bf9a821c543b7f..5482a64c81c95a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -3888,6 +3888,7 @@ static int ice_vsi_set_dflt_rss_lut(struct ice_vsi
> *vsi, int req_rss_size)  static int ice_set_channels(struct net_device
> *dev, struct ethtool_channels *ch)  {
>  	struct ice_netdev_priv *np = netdev_priv(dev);
> +	struct auxiliary_device *adev = NULL;
>  	struct ice_vsi *vsi = np->vsi;
>  	struct ice_pf *pf = vsi->back;
>  	int new_rx = 0, new_tx = 0;
> @@ -3942,14 +3943,19 @@ static int ice_set_channels(struct net_device
> *dev, struct ethtool_channels *ch)
>  		return -EINVAL;
>  	}
> 
> -	if (pf->cdev_info && pf->cdev_info->adev) {
> +	if (pf->cdev_info) {
>  		mutex_lock(&pf->adev_mutex);
> -		device_lock(&pf->cdev_info->adev->dev);
> -		locked = true;
> -		if (pf->cdev_info->adev->dev.driver) {
> -			netdev_err(dev, "Cannot change channels when RDMA
> is active\n");
> -			ret = -EBUSY;
> -			goto adev_unlock;
> +		adev = pf->cdev_info->adev;
> +		if (adev) {
> +			device_lock(&adev->dev);
> +			locked = true;
> +			if (adev->dev.driver) {
> +				netdev_err(dev, "Cannot change channels when
> RDMA is active\n");
> +				ret = -EBUSY;
> +				goto adev_unlock;
> +			}
> +		} else {
> +			mutex_unlock(&pf->adev_mutex);
>  		}
>  	}
> 
> @@ -3965,7 +3971,7 @@ static int ice_set_channels(struct net_device
> *dev, struct ethtool_channels *ch)
> 
>  adev_unlock:
>  	if (locked) {
> -		device_unlock(&pf->cdev_info->adev->dev);
> +		device_unlock(&adev->dev);
>  		mutex_unlock(&pf->adev_mutex);
>  	}
>  	return ret;
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index d88835482d3aac..984479f1e1f6d9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -9406,6 +9406,7 @@ ice_setup_tc(struct net_device *netdev, enum
> tc_setup_type type,
>  	struct ice_netdev_priv *np = netdev_priv(netdev);
>  	enum flow_block_binder_type binder_type;
>  	struct iidc_rdma_core_dev_info *cdev;
> +	struct auxiliary_device *adev = NULL;
>  	struct ice_pf *pf = np->vsi->back;
>  	flow_setup_cb_t *flower_handler;
>  	bool locked = false;
> @@ -9438,14 +9439,19 @@ ice_setup_tc(struct net_device *netdev, enum
> tc_setup_type type,
>  		}
> 
>  		cdev = pf->cdev_info;
> -		if (cdev && cdev->adev) {
> +		if (cdev) {
>  			mutex_lock(&pf->adev_mutex);
> -			device_lock(&cdev->adev->dev);
> -			locked = true;
> -			if (cdev->adev->dev.driver) {
> -				netdev_err(netdev, "Cannot change qdisc when
> RDMA is active\n");
> -				err = -EBUSY;
> -				goto adev_unlock;
> +			adev = cdev->adev;
> +			if (adev) {
> +				device_lock(&adev->dev);
> +				locked = true;
> +				if (adev->dev.driver) {
> +					netdev_err(netdev, "Cannot change
> qdisc when RDMA is active\n");
> +					err = -EBUSY;
> +					goto adev_unlock;
> +				}
> +			} else {
> +				mutex_unlock(&pf->adev_mutex);
>  			}
>  		}
> 
> @@ -9456,7 +9462,7 @@ ice_setup_tc(struct net_device *netdev, enum
> tc_setup_type type,
> 
>  adev_unlock:
>  		if (locked) {
> -			device_unlock(&cdev->adev->dev);
> +			device_unlock(&adev->dev);
>  			mutex_unlock(&pf->adev_mutex);
>  		}
>  		return err;
> --
> 2.56.0

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@xxxxxxxxx>