[Intel-wired-lan] [PATCH iwl-net] idpf: add read memory barrier when checking descriptor done bit
Add read memory barrier to ensure the order of operations when accessing
control queue descriptors. Specifically, we want to avoid cases where loads
can be reordered:
1. Load #1 is dispatched to read descriptor flags.
2. Load #2 is dispatched to read some other field from the descriptor.
3. Load #2 completes, accessing memory/cache at a point in time when the DD
flag is zero.
4. NIC DMA overwrites the descriptor, now the DD flag is one.
5. Any fields loaded before step 4 are now inconsistent with the actual
descriptor state.
Add read memory barrier between steps 1 and 2, so that load #2 is not
executed until load has completed.
Fixes: 8077c727561a ("idpf: add controlq init and reset checks")
Reviewed-by: Przemek Kitszel
Reviewed-by: Sridhar Samudrala
Suggested-by: Lance Richardson
Signed-off-by: Emil Tantilov
---
drivers/net/ethernet/intel/idpf/idpf_controlq.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_controlq.c
b/drivers/net/ethernet/intel/idpf/idpf_controlq.c
index 4849590a5591..61c7fafa54a1 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_controlq.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_controlq.c
@@ -375,6 +375,11 @@ int idpf_ctlq_clean_sq(struct idpf_ctlq_info *cq, u16
*clean_count,
desc = IDPF_CTLQ_DESC(cq, ntc);
if (!(le16_to_cpu(desc->flags) & IDPF_CTLQ_FLAG_DD))
break;
+ /*
+* This barrier is needed to ensure that no other fields
+* are read until we check the DD flag.
+*/
+ dma_rmb();
/* strip off FW internal code */
desc_err = le16_to_cpu(desc->ret_val) & 0xff;
@@ -562,6 +567,11 @@ int idpf_ctlq_recv(struct idpf_ctlq_info *cq, u16
*num_q_msg,
if (!(flags & IDPF_CTLQ_FLAG_DD))
break;
+ /*
+* This barrier is needed to ensure that no other fields
+* are read until we check the DD flag.
+*/
+ dma_rmb();
q_msg[i].vmvf_type = (flags &
(IDPF_CTLQ_FLAG_FTYPE_VM |
--
2.17.2
[Intel-wired-lan] [PATCH iwl-net] idpf: check error for register_netdev() on init
Current init logic ignores the error code from register_netdev(),
which will cause WARN_ON() on attempt to unregister it, if there was one,
and there is no info for the user that the creation of the netdev failed.
WARNING: CPU: 89 PID: 6902 at net/core/dev.c:11512
unregister_netdevice_many_notify+0x211/0x1a10
...
[ 3707.563641] unregister_netdev+0x1c/0x30
[ 3707.563656] idpf_vport_dealloc+0x5cf/0xce0 [idpf]
[ 3707.563684] idpf_deinit_task+0xef/0x160 [idpf]
[ 3707.563712] idpf_vc_core_deinit+0x84/0x320 [idpf]
[ 3707.563739] idpf_remove+0xbf/0x780 [idpf]
[ 3707.563769] pci_device_remove+0xab/0x1e0
[ 3707.563786] device_release_driver_internal+0x371/0x530
[ 3707.563803] driver_detach+0xbf/0x180
[ 3707.563816] bus_remove_driver+0x11b/0x2a0
[ 3707.563829] pci_unregister_driver+0x2a/0x250
Introduce an error check and log the vport number and error code.
On removal make sure to check VPORT_REG_NETDEV flag prior to calling
unregister and free on the netdev.
Add local variables for idx, vport_config and netdev for readability.
Fixes: 0fe45467a104 ("idpf: add create vport and netdev configuration")
Reviewed-by: Madhu Chittim
Suggested-by: Tony Nguyen
Signed-off-by: Emil Tantilov
---
drivers/net/ethernet/intel/idpf/idpf_lib.c | 27 ++
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c
b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index a3d6b8f198a8..a322a8ac771e 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -927,15 +927,19 @@ static int idpf_stop(struct net_device *netdev)
static void idpf_decfg_netdev(struct idpf_vport *vport)
{
struct idpf_adapter *adapter = vport->adapter;
+ u16 idx = vport->idx;
kfree(vport->rx_ptype_lkup);
vport->rx_ptype_lkup = NULL;
- unregister_netdev(vport->netdev);
- free_netdev(vport->netdev);
+ if (test_and_clear_bit(IDPF_VPORT_REG_NETDEV,
+ adapter->vport_config[idx]->flags)) {
+ unregister_netdev(vport->netdev);
+ free_netdev(vport->netdev);
+ }
vport->netdev = NULL;
- adapter->netdevs[vport->idx] = NULL;
+ adapter->netdevs[idx] = NULL;
}
/**
@@ -1536,12 +1540,17 @@ void idpf_init_task(struct work_struct *work)
}
for (index = 0; index < adapter->max_vports; index++) {
- if (adapter->netdevs[index] &&
- !test_bit(IDPF_VPORT_REG_NETDEV,
- adapter->vport_config[index]->flags)) {
- register_netdev(adapter->netdevs[index]);
- set_bit(IDPF_VPORT_REG_NETDEV,
- adapter->vport_config[index]->flags);
+ struct idpf_vport_config *vport_config =
adapter->vport_config[index];
+ struct net_device *netdev = adapter->netdevs[index];
+
+ if (netdev && !test_bit(IDPF_VPORT_REG_NETDEV,
vport_config->flags)) {
+ err = register_netdev(netdev);
+ if (err) {
+ dev_err(&pdev->dev, "failed to register netdev
for vport %d: %pe\n",
+ index, ERR_PTR(err));
+ continue;
+ }
+ set_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags);
}
}
--
2.17.2
[Intel-wired-lan] [PATCH iwl-net] idpf: fix transaction timeouts on reset
Restore the call to idpf_vc_xn_shutdown() at the beginning of
idpf_vc_core_deinit() provided the function is not called on
remove. In the reset path this call is needed to prevent mailbox
transactions from timing out.
Fixes: 09d0fb5cb30e ("idpf: deinit virtchnl transaction manager after vport and
vectors")
Reviewed-by: Larysa Zaremba
Signed-off-by: Emil Tantilov
---
Testing hints:
echo 1 > /sys/class/net//device/reset
---
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index d46c95f91b0d..0387794daf17 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -3080,9 +3080,15 @@ void idpf_vc_core_deinit(struct idpf_adapter *adapter)
if (!test_bit(IDPF_VC_CORE_INIT, adapter->flags))
return;
+ /* Avoid transaction timeouts when called during reset */
+ if (!test_bit(IDPF_REMOVE_IN_PROG, adapter->flags))
+ idpf_vc_xn_shutdown(adapter->vcxn_mngr);
+
idpf_deinit_task(adapter);
idpf_intr_rel(adapter);
- idpf_vc_xn_shutdown(adapter->vcxn_mngr);
+
+ if (test_bit(IDPF_REMOVE_IN_PROG, adapter->flags))
+ idpf_vc_xn_shutdown(adapter->vcxn_mngr);
cancel_delayed_work_sync(&adapter->serv_task);
cancel_delayed_work_sync(&adapter->mbx_task);
--
2.17.2
[Intel-wired-lan] [PATCH iwl-net v2] idpf: fix transaction timeouts on reset
Restore the call to idpf_vc_xn_shutdown() at the beginning of
idpf_vc_core_deinit() provided the function is not called on remove.
In the reset path the mailbox is destroyed, leading to all transactions
timing out.
Fixes: 09d0fb5cb30e ("idpf: deinit virtchnl transaction manager after vport and
vectors")
Reviewed-by: Larysa Zaremba
Signed-off-by: Emil Tantilov
---
Changelog:
v2:
- Assigned the current state of REMOVE_IN_PROG flag to a boolean
variable, to be checked instead of reading the flag twice.
- Updated the description to clarify the reason for the timeouts on
reset is due to the mailbox being destroyed.
v1:
https://lore.kernel.org/intel-wired-lan/20241218014417.3786-1-emil.s.tanti...@intel.com/
Testing hints:
echo 1 > /sys/class/net//device/reset
---
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index d46c95f91b0d..7639d520b806 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -3077,12 +3077,21 @@ int idpf_vc_core_init(struct idpf_adapter *adapter)
*/
void idpf_vc_core_deinit(struct idpf_adapter *adapter)
{
+ bool remove_in_prog;
+
if (!test_bit(IDPF_VC_CORE_INIT, adapter->flags))
return;
+ /* Avoid transaction timeouts when called during reset */
+ remove_in_prog = test_bit(IDPF_REMOVE_IN_PROG, adapter->flags);
+ if (!remove_in_prog)
+ idpf_vc_xn_shutdown(adapter->vcxn_mngr);
+
idpf_deinit_task(adapter);
idpf_intr_rel(adapter);
- idpf_vc_xn_shutdown(adapter->vcxn_mngr);
+
+ if (remove_in_prog)
+ idpf_vc_xn_shutdown(adapter->vcxn_mngr);
cancel_delayed_work_sync(&adapter->serv_task);
cancel_delayed_work_sync(&adapter->mbx_task);
--
2.17.2
[Intel-wired-lan] [PATCH iwl-net v2] idpf: add read memory barrier when checking descriptor done bit
Add read memory barrier to ensure the order of operations when accessing
control queue descriptors. Specifically, we want to avoid cases where loads
can be reordered:
1. Load #1 is dispatched to read descriptor flags.
2. Load #2 is dispatched to read some other field from the descriptor.
3. Load #2 completes, accessing memory/cache at a point in time when the DD
flag is zero.
4. NIC DMA overwrites the descriptor, now the DD flag is one.
5. Any fields loaded before step 4 are now inconsistent with the actual
descriptor state.
Add read memory barrier between steps 1 and 2, so that load #2 is not
executed until load #1 has completed.
Fixes: 8077c727561a ("idpf: add controlq init and reset checks")
Reviewed-by: Przemek Kitszel
Reviewed-by: Sridhar Samudrala
Suggested-by: Lance Richardson
Signed-off-by: Emil Tantilov
---
Changelog
v2:
- Rewrote comment to fit on a single line
- Added new line as separator
- Updated last sentence in commit message to include load #
v1:
https://lore.kernel.org/intel-wired-lan/20241115021618.20565-1-emil.s.tanti...@intel.com/
---
drivers/net/ethernet/intel/idpf/idpf_controlq.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_controlq.c
b/drivers/net/ethernet/intel/idpf/idpf_controlq.c
index 4849590a5591..b28991dd1870 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_controlq.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_controlq.c
@@ -376,6 +376,9 @@ int idpf_ctlq_clean_sq(struct idpf_ctlq_info *cq, u16
*clean_count,
if (!(le16_to_cpu(desc->flags) & IDPF_CTLQ_FLAG_DD))
break;
+ /* Ensure no other fields are read until DD flag is checked */
+ dma_rmb();
+
/* strip off FW internal code */
desc_err = le16_to_cpu(desc->ret_val) & 0xff;
@@ -563,6 +566,9 @@ int idpf_ctlq_recv(struct idpf_ctlq_info *cq, u16
*num_q_msg,
if (!(flags & IDPF_CTLQ_FLAG_DD))
break;
+ /* Ensure no other fields are read until DD flag is checked */
+ dma_rmb();
+
q_msg[i].vmvf_type = (flags &
(IDPF_CTLQ_FLAG_FTYPE_VM |
IDPF_CTLQ_FLAG_FTYPE_PF)) >>
--
2.17.2
[Intel-wired-lan] [PATCH iwl-net v2] idpf: fix adapter NULL pointer dereference on reboot
With SRIOV enabled, idpf ends up calling into idpf_remove() twice.
First via idpf_shutdown() and then again when idpf_remove() calls into
sriov_disable(), because the VF devices use the idpf driver, hence the
same remove routine. When that happens, it is possible for the adapter
to be NULL from the first call to idpf_remove(), leading to a NULL
pointer dereference.
echo 1 > /sys/class/net//device/sriov_numvfs
reboot
BUG: kernel NULL pointer dereference, address: 0020
...
RIP: 0010:idpf_remove+0x22/0x1f0 [idpf]
...
? idpf_remove+0x22/0x1f0 [idpf]
? idpf_remove+0x1e4/0x1f0 [idpf]
pci_device_remove+0x3f/0xb0
device_release_driver_internal+0x19f/0x200
pci_stop_bus_device+0x6d/0x90
pci_stop_and_remove_bus_device+0x12/0x20
pci_iov_remove_virtfn+0xbe/0x120
sriov_disable+0x34/0xe0
idpf_sriov_configure+0x58/0x140 [idpf]
idpf_remove+0x1b9/0x1f0 [idpf]
idpf_shutdown+0x12/0x30 [idpf]
pci_device_shutdown+0x35/0x60
device_shutdown+0x156/0x200
...
Replace the direct idpf_remove() call in idpf_shutdown() with
idpf_vc_core_deinit() and idpf_deinit_dflt_mbx(), which perform
the bulk of the cleanup, such as stopping the init task, freeing IRQs,
destroying the vports and freeing the mailbox. This avoids the calls to
sriov_disable() in addition to a small netdev cleanup, and destroying
workqueues, which don't seem to be required on shutdown.
Reported-by: Yuying Ma
Fixes: e850efed5e15 ("idpf: add module register and probe functionality")
Reviewed-by: Madhu Chittim
Signed-off-by: Emil Tantilov
---
Changelog:
v2:
- Updated the description to clarify the path leading up to the crash,
and the difference in the logic between remove and shutdown as result
of this change.
v1:
https://lore.kernel.org/intel-wired-lan/20250307003956.22018-1-emil.s.tanti...@intel.com/
---
drivers/net/ethernet/intel/idpf/idpf_main.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c
b/drivers/net/ethernet/intel/idpf/idpf_main.c
index b6c515d14cbf..bec4a02c5373 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_main.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
@@ -87,7 +87,11 @@ static void idpf_remove(struct pci_dev *pdev)
*/
static void idpf_shutdown(struct pci_dev *pdev)
{
- idpf_remove(pdev);
+ struct idpf_adapter *adapter = pci_get_drvdata(pdev);
+
+ cancel_delayed_work_sync(&adapter->vc_event_task);
+ idpf_vc_core_deinit(adapter);
+ idpf_deinit_dflt_mbx(adapter);
if (system_state == SYSTEM_POWER_OFF)
pci_set_power_state(pdev, PCI_D3hot);
--
2.17.2
[Intel-wired-lan] [PATCH iwl-net] idpf: fix adapter NULL pointer dereference on reboot
Driver calls idpf_remove() from idpf_shutdown(), which can end up
calling idpf_remove() again when disabling SRIOV.
echo 1 > /sys/class/net//device/sriov_numvfs
reboot
BUG: kernel NULL pointer dereference, address: 0020
...
RIP: 0010:idpf_remove+0x22/0x1f0 [idpf]
...
? idpf_remove+0x22/0x1f0 [idpf]
? idpf_remove+0x1e4/0x1f0 [idpf]
pci_device_remove+0x3f/0xb0
device_release_driver_internal+0x19f/0x200
pci_stop_bus_device+0x6d/0x90
pci_stop_and_remove_bus_device+0x12/0x20
pci_iov_remove_virtfn+0xbe/0x120
sriov_disable+0x34/0xe0
idpf_sriov_configure+0x58/0x140 [idpf]
idpf_remove+0x1b9/0x1f0 [idpf]
idpf_shutdown+0x12/0x30 [idpf]
pci_device_shutdown+0x35/0x60
device_shutdown+0x156/0x200
...
Replace the direct idpf_remove() call in idpf_shutdown() with
idpf_vc_core_deinit() and idpf_deinit_dflt_mbx(), which perform
the bulk of the cleanup, such as stopping the init task, freeing IRQs,
destroying the vports and freeing the mailbox.
Reported-by: Yuying Ma
Fixes: e850efed5e15 ("idpf: add module register and probe functionality")
Reviewed-by: Madhu Chittim
Signed-off-by: Emil Tantilov
---
drivers/net/ethernet/intel/idpf/idpf_main.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c
b/drivers/net/ethernet/intel/idpf/idpf_main.c
index b6c515d14cbf..bec4a02c5373 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_main.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
@@ -87,7 +87,11 @@ static void idpf_remove(struct pci_dev *pdev)
*/
static void idpf_shutdown(struct pci_dev *pdev)
{
- idpf_remove(pdev);
+ struct idpf_adapter *adapter = pci_get_drvdata(pdev);
+
+ cancel_delayed_work_sync(&adapter->vc_event_task);
+ idpf_vc_core_deinit(adapter);
+ idpf_deinit_dflt_mbx(adapter);
if (system_state == SYSTEM_POWER_OFF)
pci_set_power_state(pdev, PCI_D3hot);
--
2.17.2
[Intel-wired-lan] [PATCH iwl-net v2] idpf: check error for register_netdev() on init
Current init logic ignores the error code from register_netdev(),
which will cause WARN_ON() on attempt to unregister it, if there was one,
and there is no info for the user that the creation of the netdev failed.
WARNING: CPU: 89 PID: 6902 at net/core/dev.c:11512
unregister_netdevice_many_notify+0x211/0x1a10
...
[ 3707.563641] unregister_netdev+0x1c/0x30
[ 3707.563656] idpf_vport_dealloc+0x5cf/0xce0 [idpf]
[ 3707.563684] idpf_deinit_task+0xef/0x160 [idpf]
[ 3707.563712] idpf_vc_core_deinit+0x84/0x320 [idpf]
[ 3707.563739] idpf_remove+0xbf/0x780 [idpf]
[ 3707.563769] pci_device_remove+0xab/0x1e0
[ 3707.563786] device_release_driver_internal+0x371/0x530
[ 3707.563803] driver_detach+0xbf/0x180
[ 3707.563816] bus_remove_driver+0x11b/0x2a0
[ 3707.563829] pci_unregister_driver+0x2a/0x250
Introduce an error check and log the vport number and error code.
On removal make sure to check VPORT_REG_NETDEV flag prior to calling
unregister and free on the netdev.
Add local variables for idx, vport_config and netdev for readability.
Fixes: 0fe45467a104 ("idpf: add create vport and netdev configuration")
Suggested-by: Tony Nguyen
Signed-off-by: Emil Tantilov
---
Changelog:
v2:
- Refactored a bit to avoid >80 char lines.
- Changed the netdev and flag check to allow for early continue in the
max_vports loop, which also helps to reduce the identation.
v1:
https://lore.kernel.org/intel-wired-lan/20250211023851.21090-1-emil.s.tanti...@intel.com/
---
drivers/net/ethernet/intel/idpf/idpf_lib.c | 31 +++---
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c
b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index a3d6b8f198a8..a055a47449f1 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -927,15 +927,19 @@ static int idpf_stop(struct net_device *netdev)
static void idpf_decfg_netdev(struct idpf_vport *vport)
{
struct idpf_adapter *adapter = vport->adapter;
+ u16 idx = vport->idx;
kfree(vport->rx_ptype_lkup);
vport->rx_ptype_lkup = NULL;
- unregister_netdev(vport->netdev);
- free_netdev(vport->netdev);
+ if (test_and_clear_bit(IDPF_VPORT_REG_NETDEV,
+ adapter->vport_config[idx]->flags)) {
+ unregister_netdev(vport->netdev);
+ free_netdev(vport->netdev);
+ }
vport->netdev = NULL;
- adapter->netdevs[vport->idx] = NULL;
+ adapter->netdevs[idx] = NULL;
}
/**
@@ -1536,13 +1540,22 @@ void idpf_init_task(struct work_struct *work)
}
for (index = 0; index < adapter->max_vports; index++) {
- if (adapter->netdevs[index] &&
- !test_bit(IDPF_VPORT_REG_NETDEV,
- adapter->vport_config[index]->flags)) {
- register_netdev(adapter->netdevs[index]);
- set_bit(IDPF_VPORT_REG_NETDEV,
- adapter->vport_config[index]->flags);
+ struct net_device *netdev = adapter->netdevs[index];
+ struct idpf_vport_config *vport_config;
+
+ vport_config = adapter->vport_config[index];
+
+ if (!netdev ||
+ test_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags))
+ continue;
+
+ err = register_netdev(netdev);
+ if (err) {
+ dev_err(&pdev->dev, "failed to register netdev for
vport %d: %pe\n",
+ index, ERR_PTR(err));
+ continue;
}
+ set_bit(IDPF_VPORT_REG_NETDEV, vport_config->flags);
}
/* As all the required vports are created, clear the reset flag
--
2.17.2
[Intel-wired-lan] [PATCH iwl-net] idpf: avoid mailbox timeout delays during reset
Mailbox operations are not possible while the driver is in reset.
Operations that require MBX exchange with the control plane will result
in long delays if executed while a reset is in progress:
ethtool -L combined 8& echo 1 > /sys/class/net//device/reset
idpf :83:00.0: HW reset detected
idpf :83:00.0: Device HW Reset initiated
idpf :83:00.0: Transaction timed-out (op:504 cookie:be00 vc_op:504 salt:be
timeout:2000ms)
idpf :83:00.0: Transaction timed-out (op:508 cookie:bf00 vc_op:508 salt:bf
timeout:2000ms)
idpf :83:00.0: Transaction timed-out (op:512 cookie:c000 vc_op:512 salt:c0
timeout:2000ms)
idpf :83:00.0: Transaction timed-out (op:510 cookie:c100 vc_op:510 salt:c1
timeout:2000ms)
idpf :83:00.0: Transaction timed-out (op:509 cookie:c200 vc_op:509 salt:c2
timeout:6ms)
idpf :83:00.0: Transaction timed-out (op:509 cookie:c300 vc_op:509 salt:c3
timeout:6ms)
idpf :83:00.0: Transaction timed-out (op:505 cookie:c400 vc_op:505 salt:c4
timeout:6ms)
idpf :83:00.0: Failed to configure queues for vport 0, -62
Disable mailbox communication in case of a reset, unless it's done during
a driver load, where the virtchnl operations are needed to configure the
device.
Fixes: 8077c727561aa ("idpf: add controlq init and reset checks")
Co-developed-by: Joshua Hay
Signed-off-by: Joshua Hay
Signed-off-by: Emil Tantilov
Reviewed-by: Ahmed Zaki
---
drivers/net/ethernet/intel/idpf/idpf_lib.c | 18 +-
.../net/ethernet/intel/idpf/idpf_virtchnl.c| 2 +-
.../net/ethernet/intel/idpf/idpf_virtchnl.h| 1 +
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c
b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 3a033ce19cda..2ed801398971 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -1816,11 +1816,19 @@ void idpf_vc_event_task(struct work_struct *work)
if (test_bit(IDPF_REMOVE_IN_PROG, adapter->flags))
return;
- if (test_bit(IDPF_HR_FUNC_RESET, adapter->flags) ||
- test_bit(IDPF_HR_DRV_LOAD, adapter->flags)) {
- set_bit(IDPF_HR_RESET_IN_PROG, adapter->flags);
- idpf_init_hard_reset(adapter);
- }
+ if (test_bit(IDPF_HR_FUNC_RESET, adapter->flags))
+ goto func_reset;
+
+ if (test_bit(IDPF_HR_DRV_LOAD, adapter->flags))
+ goto drv_load;
+
+ return;
+
+func_reset:
+ idpf_vc_xn_shutdown(adapter->vcxn_mngr);
+drv_load:
+ set_bit(IDPF_HR_RESET_IN_PROG, adapter->flags);
+ idpf_init_hard_reset(adapter);
}
/**
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index 3d2413b8684f..5d2ca007f682 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -376,7 +376,7 @@ static void idpf_vc_xn_init(struct idpf_vc_xn_manager
*vcxn_mngr)
* All waiting threads will be woken-up and their transaction aborted. Further
* operations on that object will fail.
*/
-static void idpf_vc_xn_shutdown(struct idpf_vc_xn_manager *vcxn_mngr)
+void idpf_vc_xn_shutdown(struct idpf_vc_xn_manager *vcxn_mngr)
{
int i;
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
index 83da5d8da56b..23271cf0a216 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
@@ -66,5 +66,6 @@ int idpf_send_get_stats_msg(struct idpf_vport *vport);
int idpf_send_set_sriov_vfs_msg(struct idpf_adapter *adapter, u16 num_vfs);
int idpf_send_get_set_rss_key_msg(struct idpf_vport *vport, bool get);
int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get);
+void idpf_vc_xn_shutdown(struct idpf_vc_xn_manager *vcxn_mngr);
#endif /* _IDPF_VIRTCHNL_H_ */
--
2.17.2
[Intel-wired-lan] [PATCH iwl-net 2/2] ice: fix possible leak in ice_plug_aux_dev() error path
Fix a memory leak in the error path where kfree(iadev) was not called
following an error in auxiliary_device_add().
Fixes: f9f5301e7e2d ("ice: Register auxiliary device to provide RDMA")
Signed-off-by: Emil Tantilov
Reviewed-by: Przemek Kitszel
Reviewed-by: Aleksandr Loktionov
---
drivers/net/ethernet/intel/ice/ice_idc.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_idc.c
b/drivers/net/ethernet/intel/ice/ice_idc.c
index 420d45c2558b..8c4a3dc22a7c 100644
--- a/drivers/net/ethernet/intel/ice/ice_idc.c
+++ b/drivers/net/ethernet/intel/ice/ice_idc.c
@@ -322,16 +322,12 @@ int ice_plug_aux_dev(struct ice_pf *pf)
"roce" : "iwarp";
ret = auxiliary_device_init(adev);
- if (ret) {
- kfree(iadev);
- return ret;
- }
+ if (ret)
+ goto free_iadev;
ret = auxiliary_device_add(adev);
- if (ret) {
- auxiliary_device_uninit(adev);
- return ret;
- }
+ if (ret)
+ goto aux_dev_uninit;
mutex_lock(&pf->adev_mutex);
cdev->adev = adev;
@@ -339,6 +335,13 @@ int ice_plug_aux_dev(struct ice_pf *pf)
set_bit(ICE_FLAG_AUX_DEV_CREATED, pf->flags);
return 0;
+
+aux_dev_uninit:
+ auxiliary_device_uninit(adev);
+free_iadev:
+ kfree(iadev);
+
+ return ret;
}
/* ice_unplug_aux_dev - unregister and free AUX device
--
2.37.3
[Intel-wired-lan] [PATCH iwl-net 0/2] ice: Fix NULL pointer dereference and leak in IDC
Resolve NULL pointer dereference on driver load when RDMA is not supported. During reset the driver attempts to remove an auxiliary device that does not exist. In addition, fix a memory leak in ice_plug_aux_dev() error path. Emil Tantilov (2): ice: fix NULL pointer dereference in ice_unplug_aux_dev() on reset ice: fix possible leak in ice_plug_aux_dev() error path drivers/net/ethernet/intel/ice/ice.h | 1 + drivers/net/ethernet/intel/ice/ice_idc.c | 29 ++-- 2 files changed, 18 insertions(+), 12 deletions(-) -- 2.37.3
[Intel-wired-lan] [PATCH iwl-net 1/2] ice: fix NULL pointer dereference in ice_unplug_aux_dev() on reset
Issuing a reset when the driver is loaded without RDMA support, will
results in a crash as it attempts to remove RDMA's non-existent auxbus
device:
echo 1 > /sys/class/net//device/reset
BUG: kernel NULL pointer dereference, address: 0008
...
RIP: 0010:ice_unplug_aux_dev+0x29/0x70 [ice]
...
Call Trace:
ice_prepare_for_reset+0x77/0x260 [ice]
pci_dev_save_and_disable+0x2c/0x70
pci_reset_function+0x88/0x130
reset_store+0x5a/0xa0
kernfs_fop_write_iter+0x15e/0x210
vfs_write+0x273/0x520
ksys_write+0x6b/0xe0
do_syscall_64+0x79/0x3b0
entry_SYSCALL_64_after_hwframe+0x76/0x7e
ice_unplug_aux_dev() checks pf->cdev_info->adev for NULL pointer, but
pf->cdev_info will also be NULL, leading to the deref in the trace above.
Introduce a flag to be set when the creation of the auxbus device is
successful, to avoid multiple NULL pointer checks in ice_unplug_aux_dev().
Fixes: c24a65b6a27c7 ("iidc/ice/irdma: Update IDC to support multiple
consumers")
Signed-off-by: Emil Tantilov
Reviewed-by: Przemek Kitszel
---
drivers/net/ethernet/intel/ice/ice.h | 1 +
drivers/net/ethernet/intel/ice/ice_idc.c | 10 ++
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice.h
b/drivers/net/ethernet/intel/ice/ice.h
index ddd0ad68185b..0ef11b7ab477 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -509,6 +509,7 @@ enum ice_pf_flags {
ICE_FLAG_LINK_LENIENT_MODE_ENA,
ICE_FLAG_PLUG_AUX_DEV,
ICE_FLAG_UNPLUG_AUX_DEV,
+ ICE_FLAG_AUX_DEV_CREATED,
ICE_FLAG_MTU_CHANGED,
ICE_FLAG_GNSS, /* GNSS successfully initialized */
ICE_FLAG_DPLL, /* SyncE/PTP dplls initialized */
diff --git a/drivers/net/ethernet/intel/ice/ice_idc.c
b/drivers/net/ethernet/intel/ice/ice_idc.c
index 6ab53e430f91..420d45c2558b 100644
--- a/drivers/net/ethernet/intel/ice/ice_idc.c
+++ b/drivers/net/ethernet/intel/ice/ice_idc.c
@@ -336,6 +336,7 @@ int ice_plug_aux_dev(struct ice_pf *pf)
mutex_lock(&pf->adev_mutex);
cdev->adev = adev;
mutex_unlock(&pf->adev_mutex);
+ set_bit(ICE_FLAG_AUX_DEV_CREATED, pf->flags);
return 0;
}
@@ -347,15 +348,16 @@ void ice_unplug_aux_dev(struct ice_pf *pf)
{
struct auxiliary_device *adev;
+ if (!test_and_clear_bit(ICE_FLAG_AUX_DEV_CREATED, pf->flags))
+ return;
+
mutex_lock(&pf->adev_mutex);
adev = pf->cdev_info->adev;
pf->cdev_info->adev = NULL;
mutex_unlock(&pf->adev_mutex);
- if (adev) {
- auxiliary_device_delete(adev);
- auxiliary_device_uninit(adev);
- }
+ auxiliary_device_delete(adev);
+ auxiliary_device_uninit(adev);
}
/**
--
2.37.3
