Re: [PATCH iwl-net v3 4/4] ice: do not block on the aux device lock in ndo_setup_tc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] ice: orphaned channel VSIs and memory leak when ice_setup_tc()
  fails teardown
--

commit e098adc9e591059f39322dbf2abaa105d7a546c3
Author: Petr Oros <poros@redhat.com>

ice: do not block on the aux device lock in ndo_setup_tc

This commit replaces device_lock() with device_trylock() on the RDMA
auxiliary device in the TC_SETUP_QDISC_MQPRIO path of ice_setup_tc() to
avoid a lock ordering cycle with RTNL.

> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index bbe4409b16cf5..fbcce9bd798dc 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -9581,7 +9581,11 @@ ice_setup_tc(struct net_device *netdev, enum tc_setup_type type,
>  			mutex_lock(&pf->adev_mutex);
>  			adev = cdev->adev;
>  			if (adev) {
> -				device_lock(&adev->dev);
> +				if (!device_trylock(&adev->dev)) {
> +					mutex_unlock(&pf->adev_mutex);
> +					netdev_warn(netdev, "RDMA auxiliary device is busy, qdisc not changed\n");
> +					return -EBUSY;
> +				}

[Severity: High]
As reported by Loktionov, Aleksandr on v1 [1], does returning -EBUSY here
during qdisc destruction (qopt.hw == 0) orphan the channel VSIs and leak
vsi->ch_list?

When a user deletes or replaces an offloaded mqprio qdisc while the RDMA
auxiliary device lock is held (for example, during auxiliary driver probe
or unbind), mqprio_destroy() calls mqprio_disable_offload(), ignores the
-EBUSY return, and frees the Qdisc.  Because the Qdisc is gone, no later
qdisc teardown will run for it.

ICE_FLAG_TC_MQPRIO stays set and vsi->ch_list remains populated.  Driver
unload skips ICE_VSI_CHNL VSIs in ice_vsi_release_all() without calling
ice_remove_q_channels(), leaking the struct ice_channel entries, TC flower
channel filters, and hardware channel VSIs.  If a new offloaded mqprio
qdisc is added before then, ice_setup_tc_mqprio_qdisc() only removes
existing channels when !hw, so it appends duplicate channels to
vsi->ch_list and can index past vsi->tx_rings[] and vsi->rx_rings[] in
ice_remove_q_channels() if the new qdisc configures fewer queues.

[1] https://lore.kernel.org/r/CH3PR11MB566895E0410B0694F734EA7DAFE5662@CH3PR11MB566895.namprd11.prod.outlook.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113161453.1234851-1-poros@redhat.com?part=4