| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20241105184859.741473-4-tarun.k.singh@intel.com>
Date: Tue, 5 Nov 2024 13:48:58 -0500
From: Tarun K Singh <tarun.k.singh@...el.com>
To: intel-wired-lan@...ts.osuosl.org
Cc: netdev@...r.kernel.org
Subject: [PATCH iwl-net v1 3/4] idpf: Add init, reinit, and deinit control lock
Add new 'vport_init_lock' to prevent locking issue.
The existing 'vport_cfg_lock' was controlling the vport initialization,
re-initialization due to reset, and de-initialization of code flow.
In addition to controlling the above behavior it was also controlling
the parallel netdevice calls such as open/close from Linux OS, which
can happen independent of re-init or de-init of the vport(s). If first
one such as re-init or de-init is going on then the second operation
to config the netdevice with OS should not take place. The first
operation (init or de-init) takes the precedence if both are to happen
simultaneously.
Use of single lock cause deadlock and inconsistent behavior of code
flow. E.g. when driver undergoes reset via 'idpf_init_hard_reset', it
acquires the 'vport_cfg_lock', and during this process it tries to
unregister netdevice which may call 'idpf_stop' which tries to acquire
same lock causing it to deadlock.
To address above, add new lock 'vport_init_lock' which control the
initialization, re-initialization, and de-initialization flow.
The 'vport_cfg_lock' now exclusively controls the vport config
operations.
Add vport config lock around 'idpf_vport_stop()' and 'idpf_vport_open()'
to protect close and open operation at the same time.
Add vport init lock around 'idpf_sriv_configure()' to protect it from
load and removal path. To accomplish it, use existing function
as wrapper and introduce another function 'idpf_sriov_config_vfs'
which is used inside the lock.
In idpf_remove, change 'idpf_sriov_configure' to
'idpf_sriov_config_vfs', and move inside the init lock.
Use these two locks in the following precedence:
'vport_init_lock' first, then 'vport_cfg_lock'.
Fixes: 8077c727561a ("idpf: add controlq init and reset checks")
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
Reviewed-by: Madhu Chittim <madhu.chittim@...el.com>
Signed-off-by: Tarun K Singh <tarun.k.singh@...el.com>
---
drivers/net/ethernet/intel/idpf/idpf.h | 25 +++++++++++++
drivers/net/ethernet/intel/idpf/idpf_lib.c | 41 ++++++++++++++++++---
drivers/net/ethernet/intel/idpf/idpf_main.c | 7 +++-
3 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
index 8dea2dd784af..34dbdc7d6c88 100644
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -526,6 +526,7 @@ struct idpf_vc_xn_manager;
* @crc_enable: Enable CRC insertion offload
* @req_tx_splitq: TX split or single queue model to request
* @req_rx_splitq: RX split or single queue model to request
+ * @vport_init_lock: Lock to protect vport init, re-init, and deinit flow
* @vport_cfg_lock: Lock to protect the vport config flow
* @vector_lock: Lock to protect vector distribution
* @queue_lock: Lock to protect queue distribution
@@ -583,6 +584,7 @@ struct idpf_adapter {
bool req_tx_splitq;
bool req_rx_splitq;
+ struct mutex vport_init_lock;
struct mutex vport_cfg_lock;
struct mutex vector_lock;
struct mutex queue_lock;
@@ -786,6 +788,28 @@ static inline u16 idpf_get_max_tx_hdr_size(struct idpf_adapter *adapter)
return le16_to_cpu(adapter->caps.max_tx_hdr_size);
}
+/**
+ * idpf_vport_init_lock -acquire init/deinit control lock
+ * @adapter: private data struct
+ *
+ * It controls and protect vport initialization, re-initialization,
+ * and deinitialization code flow and its resources. This
+ * lock is only used by non-datapath code.
+ */
+static inline void idpf_vport_init_lock(struct idpf_adapter *adapter)
+{
+ mutex_lock(&adapter->vport_init_lock);
+}
+
+/**
+ * idpf_vport_init_unlock - release vport init lock
+ * @adapter: private data struct
+ */
+static inline void idpf_vport_init_unlock(struct idpf_adapter *adapter)
+{
+ mutex_unlock(&adapter->vport_init_lock);
+}
+
/**
* idpf_vport_cfg_lock - Acquire the vport config lock
* @adapter: private data struct
@@ -827,6 +851,7 @@ void idpf_set_ethtool_ops(struct net_device *netdev);
void idpf_vport_intr_write_itr(struct idpf_q_vector *q_vector,
u16 itr, bool tx);
int idpf_sriov_configure(struct pci_dev *pdev, int num_vfs);
+int idpf_sriov_config_vfs(struct pci_dev *pdev, int num_vfs);
u8 idpf_vport_get_hsplit(const struct idpf_vport *vport);
bool idpf_vport_set_hsplit(const struct idpf_vport *vport, u8 val);
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 778dc71fbf4a..931d0f988c95 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -1000,7 +1000,10 @@ static void idpf_vport_dealloc(struct idpf_vport *vport)
unsigned int i = vport->idx;
idpf_deinit_mac_addr(vport);
+
+ idpf_vport_cfg_lock(adapter);
idpf_vport_stop(vport);
+ idpf_vport_cfg_unlock(adapter);
if (!test_bit(IDPF_HR_RESET_IN_PROG, adapter->flags))
idpf_decfg_netdev(vport);
@@ -1522,8 +1525,11 @@ void idpf_init_task(struct work_struct *work)
/* Once state is put into DOWN, driver is ready for dev_open */
np = netdev_priv(vport->netdev);
np->state = __IDPF_VPORT_DOWN;
- if (test_and_clear_bit(IDPF_VPORT_UP_REQUESTED, vport_config->flags))
+ if (test_and_clear_bit(IDPF_VPORT_UP_REQUESTED, vport_config->flags)) {
+ idpf_vport_cfg_lock(adapter);
idpf_vport_open(vport);
+ idpf_vport_cfg_unlock(adapter);
+ }
/* Spawn and return 'idpf_init_task' work queue until all the
* default vports are created
@@ -1601,17 +1607,19 @@ static int idpf_sriov_ena(struct idpf_adapter *adapter, int num_vfs)
}
/**
- * idpf_sriov_configure - Configure the requested VFs
+ * idpf_sriov_config_vfs - Configure the requested VFs
* @pdev: pointer to a pci_dev structure
* @num_vfs: number of vfs to allocate
*
* Enable or change the number of VFs. Called when the user updates the number
* of VFs in sysfs.
**/
-int idpf_sriov_configure(struct pci_dev *pdev, int num_vfs)
+int idpf_sriov_config_vfs(struct pci_dev *pdev, int num_vfs)
{
struct idpf_adapter *adapter = pci_get_drvdata(pdev);
+ lockdep_assert_held(&adapter->vport_init_lock);
+
if (!idpf_is_cap_ena(adapter, IDPF_OTHER_CAPS, VIRTCHNL2_CAP_SRIOV)) {
dev_info(&pdev->dev, "SR-IOV is not supported on this device\n");
@@ -1634,6 +1642,26 @@ int idpf_sriov_configure(struct pci_dev *pdev, int num_vfs)
return 0;
}
+/**
+ * idpf_sriov_configure - Call idpf_sriov_config_vfs to configure
+ * @pdev: pointer to a pci_dev structure
+ * @num_vfs: number of vfs to allocate
+ *
+ * Enable or change the number of VFs. Called when the user updates the number
+ * of VFs in sysfs.
+ **/
+int idpf_sriov_configure(struct pci_dev *pdev, int num_vfs)
+{
+ struct idpf_adapter *adapter = pci_get_drvdata(pdev);
+ int ret;
+
+ idpf_vport_init_lock(adapter);
+ ret = idpf_sriov_config_vfs(pdev, num_vfs);
+ idpf_vport_init_unlock(adapter);
+
+ return ret;
+}
+
/**
* idpf_deinit_task - Device deinit routine
* @adapter: Driver specific private structure
@@ -1733,7 +1761,7 @@ static int idpf_init_hard_reset(struct idpf_adapter *adapter)
int err;
u16 i;
- mutex_lock(&adapter->vport_cfg_lock);
+ idpf_vport_init_lock(adapter);
dev_info(dev, "Device HW Reset initiated\n");
@@ -1798,7 +1826,7 @@ static int idpf_init_hard_reset(struct idpf_adapter *adapter)
msleep(100);
unlock_mutex:
- mutex_unlock(&adapter->vport_cfg_lock);
+ idpf_vport_init_unlock(adapter);
return err;
}
@@ -2155,6 +2183,9 @@ static int idpf_open(struct net_device *netdev)
struct idpf_vport *vport;
int err;
+ if (test_bit(IDPF_REMOVE_IN_PROG, adapter->flags))
+ return 0;
+
idpf_vport_cfg_lock(adapter);
vport = idpf_netdev_to_vport(netdev);
diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
index 0522b3a6f42c..04bbc048c829 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_main.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
@@ -28,10 +28,13 @@ static void idpf_remove(struct pci_dev *pdev)
* end up in bad state.
*/
cancel_delayed_work_sync(&adapter->vc_event_task);
+
+ idpf_vport_init_lock(adapter);
if (adapter->num_vfs)
- idpf_sriov_configure(pdev, 0);
+ idpf_sriov_config_vfs(pdev, 0);
idpf_vc_core_deinit(adapter);
+ idpf_vport_init_unlock(adapter);
/* Be a good citizen and leave the device clean on exit */
adapter->dev_ops.reg_ops.trigger_reset(adapter, IDPF_HR_FUNC_RESET);
@@ -72,6 +75,7 @@ static void idpf_remove(struct pci_dev *pdev)
kfree(adapter->vcxn_mngr);
adapter->vcxn_mngr = NULL;
+ mutex_destroy(&adapter->vport_init_lock);
mutex_destroy(&adapter->vport_cfg_lock);
mutex_destroy(&adapter->vector_lock);
mutex_destroy(&adapter->queue_lock);
@@ -229,6 +233,7 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto err_cfg_hw;
}
+ mutex_init(&adapter->vport_init_lock);
mutex_init(&adapter->vport_cfg_lock);
mutex_init(&adapter->vector_lock);
mutex_init(&adapter->queue_lock);
--
2.46.0
Powered by blists - more mailing lists