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 <[email protected]>
---
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
