* [PATCH iwl-net v4 0/2] idpf: destroy the vports on shutdown
@ 2026-10-12 6:30 Tian Xun Ng
2026-10-12 6:30 ` [PATCH iwl-net v4 1/2] idpf: don't use the Tx timestamp trackers once removal has started Tian Xun Ng
2026-10-12 6:30 ` [PATCH iwl-net v4 2/2] idpf: keep the mailbox up while tearing down vports on shutdown Tian Xun Ng
0 siblings, 2 replies; 5+ messages in thread
From: Tian Xun Ng @ 2026-10-12 6:30 UTC (permalink / raw)
To: intel-wired-lan
Cc: netdev, Emil Tantilov, anthony.l.nguyen, przemyslaw.kitszel,
aleksander.lobakin, aleksandr.loktionov, decot, milena.olech,
joshua.a.hay, andrew+netdev, davem, edumazet, kuba, pabeni,
Tian Xun Ng
From: Tian Xun Ng <tianxun.ng@bytedance.com>
On shutdown idpf shuts its virtchnl transaction manager down before the
vports are torn down, so the device is never told that they are gone.
Patch 2 keeps the mailbox up for that teardown, as remove already does,
with a short timeout so that an unresponsive control plane cannot hold
up a reboot for long. Doing so would expose shutdown to an existing
remove-path race between the async PTP Tx timestamp callback and
idpf_ptp_release(), which patch 1 closes first.
An LLM coding assistant helped analyse the problem and draft the changes
and their messages. Both patches carry an Assisted-by tag.
Changes in v4:
- New patch 1, closing the Tx timestamp callback race that Sashiko
pointed out on v3. It already affects remove; with patch 2 it would
also reach shutdown.
- Patch 2: the Tx queue SW marker wait follows the same bail-out as the
mailbox transactions, so a control plane that acks DISABLE_QUEUES but
posts no markers costs one marker timeout instead of one per queue
(Emil).
- Patch 2: the statistics task stops sending once IDPF_REMOVE_IN_PROG is
set (Emil).
- Patch 2: the shutdown comment now says that a transaction already in
flight keeps its own timeout; bounding that would need support in
libie.
- Not changed: if one teardown reply or Tx queue marker takes longer
than 500 ms, the rest of the teardown is skipped. On the tested PFs
the queue-related messages took at most 240 ms, and the slowest,
DEALLOC_VECTORS (about 300 ms), is sent last, after DESTROY_VPORT.
With the driver made to treat the first Tx queue marker as missing, so
that UNMAP_QUEUE_VECTOR, DESTROY_VPORT and DEALLOC_VECTORS were
skipped, 3 of 3 warm reboots were still clean on these devices. A
function reset as a fallback was patch 2 of v1; in our tests a
function reset on shutdown did not stop the faults, so it was dropped.
I can add one for when the timeout fires if that is preferred.
Testing, besides what patch 2 describes: W=1 builds of idpf and libie
with allmodconfig and allyesconfig showed no new warnings after either
patch. On one server, modprobe -r idpf and a reload completed without
warnings or IOMMU faults. These devices do not support PTP, so the path
that patch 1 changes was not exercised on hardware.
Changes in v3:
- No new flags: adapter->vc_xn_timeout_ms (60 s from probe) is lowered
to 500 ms on shutdown, and idpf_send_mb_msg() fails the rest at once
after a shortened transaction times out (Emil).
- IDPF_REMOVE_IN_PROG is set before the service and event tasks are
cancelled (Emil); the comment and message no longer state that a reset
does not clear the vports (Emil).
v3: https://lore.kernel.org/all/20261009032706.94324-1-luckilystar08@gmail.com/
v2: https://lore.kernel.org/all/20261007055741.30629-1-luckilystar08@gmail.com/
v1: https://lore.kernel.org/all/20260917105205.37561-1-luckilystar08@gmail.com/
Tian Xun Ng (2):
idpf: don't use the Tx timestamp trackers once removal has started
idpf: keep the mailbox up while tearing down vports on shutdown
drivers/net/ethernet/intel/idpf/idpf.h | 3 +
drivers/net/ethernet/intel/idpf/idpf_lib.c | 3 +
drivers/net/ethernet/intel/idpf/idpf_main.c | 11 +++
.../net/ethernet/intel/idpf/idpf_virtchnl.c | 68 ++++++++++++++++---
.../net/ethernet/intel/idpf/idpf_virtchnl.h | 1 +
.../ethernet/intel/idpf/idpf_virtchnl_ptp.c | 3 +-
6 files changed, 80 insertions(+), 9 deletions(-)
--
2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH iwl-net v4 1/2] idpf: don't use the Tx timestamp trackers once removal has started
2026-10-12 6:30 [PATCH iwl-net v4 0/2] idpf: destroy the vports on shutdown Tian Xun Ng
@ 2026-10-12 6:30 ` Tian Xun Ng
2026-10-13 6:31 ` sashiko-bot
2026-10-12 6:30 ` [PATCH iwl-net v4 2/2] idpf: keep the mailbox up while tearing down vports on shutdown Tian Xun Ng
1 sibling, 1 reply; 5+ messages in thread
From: Tian Xun Ng @ 2026-10-12 6:30 UTC (permalink / raw)
To: intel-wired-lan
Cc: netdev, Emil Tantilov, anthony.l.nguyen, przemyslaw.kitszel,
aleksander.lobakin, aleksandr.loktionov, decot, milena.olech,
joshua.a.hay, andrew+netdev, davem, edumazet, kuba, pabeni,
Tian Xun Ng
From: Tian Xun Ng <tianxun.ng@bytedance.com>
On remove, idpf_vc_core_deinit() keeps the virtchnl transaction manager
up until the vports have been torn down, so mbx_task keeps processing
replies while idpf_ptp_release() frees the Tx timestamp trackers. An
async VIRTCHNL2_OP_PTP_GET_VPORT_TX_TSTAMP reply handled in that window
reaches idpf_ptp_get_tx_tstamp_async_handler(), which reads
vport->tx_tstamp_caps and takes its lock after
idpf_ptp_release_vport_tstamp() may already have freed it.
Once IDPF_REMOVE_IN_PROG is set, wait for a mailbox task that may be
running a callback before releasing anything, and make the Tx timestamp
callback return when the bit is set. Matched replies to synchronous
transactions are not affected, so the vport teardown still receives
them; mbx_task is requeued straight away for that.
The next patch sets IDPF_REMOVE_IN_PROG on shutdown too, which would
otherwise expose shutdown to the same window.
Fixes: 1a49cf814fe1 ("idpf: add Tx timestamp flows")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Tian Xun Ng <tianxun.ng@bytedance.com>
---
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 12 +++++++++++-
drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c | 3 ++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index 1caf52706..d0db60647 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -3204,8 +3204,18 @@ void idpf_vc_core_deinit(struct idpf_adapter *adapter)
/* Avoid transaction timeouts when called during reset */
remove_in_prog = test_bit(IDPF_REMOVE_IN_PROG, adapter->flags);
- if (!remove_in_prog)
+ if (!remove_in_prog) {
libie_ctlq_xn_shutdown(adapter->xnm);
+ } else {
+ /* The transaction manager stays up for the teardown below, so
+ * async replies can still arrive. Wait for a mailbox task that
+ * may be running an async callback; callbacks that run later
+ * see IDPF_REMOVE_IN_PROG and leave the state released below
+ * alone.
+ */
+ cancel_delayed_work_sync(&adapter->mbx_task);
+ queue_delayed_work(adapter->mbx_wq, &adapter->mbx_task, 0);
+ }
idpf_ptp_release(adapter);
idpf_deinit_task(adapter);
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
index 14dba40a9..1422b30d0 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
@@ -550,7 +550,8 @@ idpf_ptp_get_tx_tstamp_async_handler(void *ctx, struct kvec *mem, int status)
u16 num_latches;
u32 vport_id;
- if (status)
+ /* The Tx timestamp trackers are released on remove and shutdown */
+ if (status || test_bit(IDPF_REMOVE_IN_PROG, adapter->flags))
return;
recv_tx_tstamp_msg = mem->iov_base;
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH iwl-net v4 2/2] idpf: keep the mailbox up while tearing down vports on shutdown
2026-10-12 6:30 [PATCH iwl-net v4 0/2] idpf: destroy the vports on shutdown Tian Xun Ng
2026-10-12 6:30 ` [PATCH iwl-net v4 1/2] idpf: don't use the Tx timestamp trackers once removal has started Tian Xun Ng
@ 2026-10-12 6:30 ` Tian Xun Ng
2026-10-13 6:31 ` sashiko-bot
1 sibling, 1 reply; 5+ messages in thread
From: Tian Xun Ng @ 2026-10-12 6:30 UTC (permalink / raw)
To: intel-wired-lan
Cc: netdev, Emil Tantilov, anthony.l.nguyen, przemyslaw.kitszel,
aleksander.lobakin, aleksandr.loktionov, decot, milena.olech,
joshua.a.hay, andrew+netdev, davem, edumazet, kuba, pabeni,
Tian Xun Ng
From: Tian Xun Ng <tianxun.ng@bytedance.com>
Since commit 4c9106f4906a ("idpf: fix adapter NULL pointer dereference
on reboot"), idpf_shutdown() calls idpf_vc_core_deinit() directly instead
of idpf_remove(), so IDPF_REMOVE_IN_PROG is not set on shutdown.
idpf_vc_core_deinit() uses that flag to decide when to shut the virtchnl
transaction manager down. Without it, libie_ctlq_xn_shutdown() runs
before idpf_deinit_task() tears the vports down, so every message sent
during that teardown (disable vport, disable queues, destroy vport)
fails at once, and the device is never told that the vports are gone.
A reset should clear that state, but on the system where this was found
the device kept the queues enabled across a warm reboot. The first queue
reconfiguration in the next kernel (udev setting the MTU) sends
VIRTCHNL2_OP_DEL_QUEUES, and the device then writes SW_MARKER TX
completions to the previous kernel's completion rings. With the IOMMU
translating, those writes fault about 19 s into every such boot:
arm-smmu-v3 arm-smmu-v3.12.auto: event: F_TRANSLATION client: 0006:01:00.0 sid: 0x30100 ssid: 0x0 iova: 0x3ef60840 ipa: 0x0
In IOMMU pass-through mode they corrupt pages the new kernel has reused.
Set IDPF_REMOVE_IN_PROG at the start of idpf_shutdown(), as idpf_remove()
does, so that the event task cannot be requeued and the vports are
destroyed while the mailbox is still up; the previous patch keeps the Tx
timestamp callback away from the state released on the way. So that an
unresponsive control plane does not hold up a reboot for long, cap each
transaction sent during the teardown at 500 ms instead of 60 s, through
a new adapter->vc_xn_timeout_ms, and fail later ones at once after the
first timeout. The SW marker that each disabled Tx queue waits for is
handled the same way: once one does not arrive, the remaining queues do
not wait for theirs. The statistics task stops sending once
IDPF_REMOVE_IN_PROG is set; a request it already has waiting keeps its
own timeout, as bounding that would need support in libie. If the
function has already been reset, for instance a VF whose PF went down
first, idpf_is_reset_detected() fails the messages without waiting.
Tested on two arm64 (64K pages) servers, each with two idpf PFs, with
this series backported to a 6.17 kernel and the IOMMU translating: 20
warm reboots without a fault. Each PF's teardown took about 1.2 s for
six mailbox transactions, with all 64 Tx queue markers received; the
slowest transaction, VIRTCHNL2_OP_DEALLOC_VECTORS, took about 300 ms.
With the driver made to skip sending the teardown messages, every reboot
faulted again.
Fixes: 4c9106f4906a ("idpf: fix adapter NULL pointer dereference on reboot")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Tian Xun Ng <tianxun.ng@bytedance.com>
---
drivers/net/ethernet/intel/idpf/idpf.h | 3 +
drivers/net/ethernet/intel/idpf/idpf_lib.c | 3 +
drivers/net/ethernet/intel/idpf/idpf_main.c | 11 ++++
.../net/ethernet/intel/idpf/idpf_virtchnl.c | 56 ++++++++++++++++---
.../net/ethernet/intel/idpf/idpf_virtchnl.h | 1 +
5 files changed, 67 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
index 470bc23c8..0739861a1 100644
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -625,6 +625,8 @@ struct idpf_vport_config {
* @asq: Send control queue info
* @arq: Receive control queue info
* @xnm: Xn transaction manager
+ * @vc_xn_timeout_ms: Upper bound for each Xn transaction timeout, shortened
+ * on shutdown and set to 0 once a shortened one timed out
* @num_avail_msix: Available number of MSIX vectors
* @num_msix_entries: Number of entries in MSIX table
* @msix_entries: MSIX table
@@ -684,6 +686,7 @@ struct idpf_adapter {
struct libie_ctlq_info *asq;
struct libie_ctlq_info *arq;
struct libie_ctlq_xn_manager *xnm;
+ u32 vc_xn_timeout_ms;
u16 num_avail_msix;
u16 num_msix_entries;
struct msix_entry *msix_entries;
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 827c795af..e4af4ebd4 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -1358,6 +1358,9 @@ void idpf_statistics_task(struct work_struct *work)
for (i = 0; i < adapter->max_vports; i++) {
struct idpf_vport *vport = adapter->vports[i];
+ if (test_bit(IDPF_REMOVE_IN_PROG, adapter->flags))
+ return;
+
if (vport && !test_bit(IDPF_HR_RESET_IN_PROG, adapter->flags))
idpf_send_get_stats_msg(netdev_priv(vport->netdev),
&vport->port_stats);
diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
index 129bccaa6..2e018d02f 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_main.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
@@ -194,8 +194,18 @@ static void idpf_shutdown(struct pci_dev *pdev)
{
struct idpf_adapter *adapter = pci_get_drvdata(pdev);
+ /* As in idpf_remove(): keep the event task from being requeued, and
+ * destroy the vports while the mailbox is still up.
+ */
+ set_bit(IDPF_REMOVE_IN_PROG, adapter->flags);
cancel_delayed_work_sync(&adapter->serv_task);
cancel_delayed_work_sync(&adapter->vc_event_task);
+
+ /* Bound the teardown transactions and Tx queue marker waits started
+ * from here on, so that an unresponsive control plane costs a single
+ * timeout. A transaction already in flight keeps its own timeout.
+ */
+ WRITE_ONCE(adapter->vc_xn_timeout_ms, IDPF_VC_XN_SHUTDOWN_TIMEOUT_MSEC);
idpf_vc_core_deinit(adapter);
idpf_deinit_dflt_mbx(adapter);
@@ -267,6 +277,7 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
adapter->req_rx_splitq = true;
adapter->pdev = pdev;
+ adapter->vc_xn_timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC;
err = idpf_dev_init(adapter, ent);
if (err) {
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index d0db60647..a97279447 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -190,20 +190,32 @@ static void idpf_prepare_ptp_mb_msg(struct idpf_adapter *adapter, u32 op,
* Cleanup the mailbox queue entries of the previously sent message to
* unmap and release the buffer.
*
+ * The transaction timeout is capped at adapter->vc_xn_timeout_ms. Once a
+ * transaction with a shortened timeout times out, later ones fail at once
+ * instead of waiting again.
+ *
* Return: 0 if the request was successful, -%EBUSY if reset is detected
- * or Tx control queue is full, other negative error code on failure.
+ * or Tx control queue is full, -%ETIMEDOUT if a shortened transaction
+ * already timed out, other negative error code on failure.
*/
int idpf_send_mb_msg(struct idpf_adapter *adapter,
struct libie_ctlq_xn_send_params *xn_params,
void *send_buf, size_t send_buf_size)
{
+ u32 timeout_ms = READ_ONCE(adapter->vc_xn_timeout_ms);
struct libie_ctlq_msg ctlq_msg = {};
+ int err = 0;
+
+ if (idpf_is_reset_detected(adapter))
+ err = -EBUSY;
+ else if (!timeout_ms)
+ err = -ETIMEDOUT;
- if (idpf_is_reset_detected(adapter)) {
+ if (err) {
if (!libie_cp_can_send_onstack(send_buf_size))
kfree(send_buf);
- return -EBUSY;
+ return err;
}
idpf_prepare_ptp_mb_msg(adapter, xn_params->chnl_opcode, &ctlq_msg);
@@ -214,10 +226,15 @@ int idpf_send_mb_msg(struct idpf_adapter *adapter,
xn_params->xnm = adapter->xnm;
xn_params->ctlq = xn_params->ctlq ? xn_params->ctlq : adapter->asq;
xn_params->rel_tx_buf = kfree;
+ xn_params->timeout_ms = min_t(u64, xn_params->timeout_ms, timeout_ms);
idpf_mb_clean(xn_params->ctlq, false);
- return libie_ctlq_xn_send(xn_params);
+ err = libie_ctlq_xn_send(xn_params);
+ if (err == -ETIMEDOUT && timeout_ms < IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC)
+ WRITE_ONCE(adapter->vc_xn_timeout_ms, 0);
+
+ return err;
}
/**
@@ -355,6 +372,33 @@ static int idpf_send_chunked_msg(struct idpf_adapter *adapter,
return 0;
}
+/**
+ * idpf_wait_for_marker - wait for TxQ marker
+ * @adapter: driver specific private structure
+ * @txq: Tx queue to wait for
+ *
+ * Return: %true if the marker was received
+ */
+static bool idpf_wait_for_marker(struct idpf_adapter *adapter,
+ struct idpf_tx_queue *txq)
+{
+ u32 timeout_ms = READ_ONCE(adapter->vc_xn_timeout_ms);
+
+ idpf_queue_set(SW_MARKER, txq);
+
+ if (!timeout_ms)
+ return false;
+
+ idpf_wait_for_sw_marker_completion(txq);
+ if (!idpf_queue_has(SW_MARKER, txq))
+ return true;
+
+ if (timeout_ms < IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC)
+ WRITE_ONCE(adapter->vc_xn_timeout_ms, 0);
+
+ return false;
+}
+
/**
* idpf_wait_for_marker_event_set - wait for software marker response for
* selected Tx queues
@@ -375,9 +419,7 @@ static int idpf_wait_for_marker_event_set(const struct idpf_queue_set *qs)
netdev = txq->netdev;
- idpf_queue_set(SW_MARKER, txq);
- idpf_wait_for_sw_marker_completion(txq);
- markers_rcvd &= !idpf_queue_has(SW_MARKER, txq);
+ markers_rcvd &= idpf_wait_for_marker(qs->adapter, txq);
break;
default:
break;
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
index 5d27805ff..911a25ad9 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.h
@@ -7,6 +7,7 @@
#include <linux/net/intel/virtchnl2.h>
#define IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC (60 * 1000)
+#define IDPF_VC_XN_SHUTDOWN_TIMEOUT_MSEC 500
struct idpf_adapter;
struct idpf_netdev_priv;
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH iwl-net v4 1/2] idpf: don't use the Tx timestamp trackers once removal has started
2026-10-12 6:30 ` [PATCH iwl-net v4 1/2] idpf: don't use the Tx timestamp trackers once removal has started Tian Xun Ng
@ 2026-10-13 6:31 ` sashiko-bot
0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-13 6:31 UTC (permalink / raw)
To: Tian Xun Ng, intel-wired-lan
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: idpf: UAF in idpf_handle_event_link during vport teardown
--
commit 5304a7984fe1d9aebd2f5a30c9206db3b232eff1
Author: Tian Xun Ng <tianxun.ng@bytedance.com>
idpf: don't use the Tx timestamp trackers once removal has started
On remove, idpf_vc_core_deinit() keeps the virtchnl transaction manager
up until the vports have been torn down, so mbx_task keeps processing
replies while idpf_ptp_release() frees the Tx timestamp trackers. An
async VIRTCHNL2_OP_PTP_GET_VPORT_TX_TSTAMP reply handled in that window
reaches idpf_ptp_get_tx_tstamp_async_handler(), which reads
vport->tx_tstamp_caps and takes its lock after
idpf_ptp_release_vport_tstamp() may already have freed it.
Once IDPF_REMOVE_IN_PROG is set, wait for a mailbox task that may be
running a callback before releasing anything, and make the Tx timestamp
callback return when the bit is set. Matched replies to synchronous
transactions are not affected, so the vport teardown still receives
them; mbx_task is requeued straight away for that.
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> index 1caf527069737..d0db6064757cf 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> @@ -3204,8 +3204,18 @@ void idpf_vc_core_deinit(struct idpf_adapter *adapter)
>
> /* Avoid transaction timeouts when called during reset */
> remove_in_prog = test_bit(IDPF_REMOVE_IN_PROG, adapter->flags);
> - if (!remove_in_prog)
> + if (!remove_in_prog) {
> libie_ctlq_xn_shutdown(adapter->xnm);
> + } else {
> + /* The transaction manager stays up for the teardown below, so
> + * async replies can still arrive. Wait for a mailbox task that
> + * may be running an async callback; callbacks that run later
> + * see IDPF_REMOVE_IN_PROG and leave the state released below
> + * alone.
> + */
> + cancel_delayed_work_sync(&adapter->mbx_task);
> + queue_delayed_work(adapter->mbx_wq, &adapter->mbx_task, 0);
[Severity: High]
Does requeuing the mailbox task here introduce a use-after-free window
during vport teardown?
While the task is running, it can process an async
VIRTCHNL2_EVENT_LINK_CHANGE message and trigger idpf_handle_event_link(),
which searches adapter->vports without locks via idpf_vid_to_vport():
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:idpf_handle_event_link() {
...
vport = idpf_vid_to_vport(adapter, le32_to_cpu(v2e->vport_id));
if (!vport) {
...
}
np = netdev_priv(vport->netdev);
np->link_speed_mbps = le32_to_cpu(v2e->link_speed);
...
}
> + }
>
> idpf_ptp_release(adapter);
> idpf_deinit_task(adapter);
Concurrently, idpf_deinit_task() iterates over vports and frees them:
drivers/net/ethernet/intel/idpf/idpf_lib.c:idpf_vport_dealloc() {
...
idpf_vport_rel(vport);
adapter->vports[i] = NULL;
...
}
Because idpf_vport_rel() calls kfree(vport) before the adapter->vports[i]
array element is set to NULL, idpf_vid_to_vport() in the concurrently
running mbx_task could find and return a pointer to freed memory, leading
to a use-after-free when idpf_handle_event_link() dereferences it.
Could idpf_handle_event_link() check for IDPF_REMOVE_IN_PROG, or could the
vports array be updated before the memory is freed?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012063027.27750-1-luckilystar08@gmail.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH iwl-net v4 2/2] idpf: keep the mailbox up while tearing down vports on shutdown
2026-10-12 6:30 ` [PATCH iwl-net v4 2/2] idpf: keep the mailbox up while tearing down vports on shutdown Tian Xun Ng
@ 2026-10-13 6:31 ` sashiko-bot
0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-13 6:31 UTC (permalink / raw)
To: Tian Xun Ng, intel-wired-lan
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] idpf: Use-After-Free in idpf_handle_event_link() due to concurrent teardown
--
commit b8311fab0c0442230fa268ded8ce6b4805cfd5c0
Author: Tian Xun Ng <tianxun.ng@bytedance.com>
idpf: keep the mailbox up while tearing down vports on shutdown
This commit sets IDPF_REMOVE_IN_PROG during shutdown to keep the mailbox up
while vports are destroyed, ensuring teardown messages reach the device. It
also bounds transaction timeouts during teardown to prevent unresponsive
control planes from delaying reboots.
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
> index 129bccaa6baa8..2e018d02f7dfe 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_main.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
> @@ -194,8 +194,18 @@ static void idpf_shutdown(struct pci_dev *pdev)
> {
> struct idpf_adapter *adapter = pci_get_drvdata(pdev);
>
> + /* As in idpf_remove(): keep the event task from being requeued, and
> + * destroy the vports while the mailbox is still up.
> + */
> + set_bit(IDPF_REMOVE_IN_PROG, adapter->flags);
> cancel_delayed_work_sync(&adapter->serv_task);
> cancel_delayed_work_sync(&adapter->vc_event_task);
> +
> + /* Bound the teardown transactions and Tx queue marker waits started
> + * from here on, so that an unresponsive control plane costs a single
> + * timeout. A transaction already in flight keeps its own timeout.
> + */
> + WRITE_ONCE(adapter->vc_xn_timeout_ms, IDPF_VC_XN_SHUTDOWN_TIMEOUT_MSEC);
> idpf_vc_core_deinit(adapter);
[Severity: High]
Does setting IDPF_REMOVE_IN_PROG here introduce a use-after-free regression
during teardown?
In idpf_vc_core_deinit(), when IDPF_REMOVE_IN_PROG is set, the mailbox task
is explicitly requeued:
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:idpf_vc_core_deinit() {
if (!remove_in_prog) {
libie_ctlq_xn_shutdown(adapter->xnm);
} else {
/* ... */
cancel_delayed_work_sync(&adapter->mbx_task);
queue_delayed_work(adapter->mbx_wq, &adapter->mbx_task, 0);
}
idpf_ptp_release(adapter);
idpf_deinit_task(adapter);
...
}
This allows the mailbox task to run concurrently with idpf_deinit_task()
which destroys the vports. If a hardware event (like a link state change)
arrives during this window, the mailbox task can call idpf_handle_event_link()
which looks up the vport locklessly:
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:idpf_handle_event_link() {
np = netdev_priv(vport->netdev);
np->link_speed_mbps = le32_to_cpu(v2e->link_speed);
if (vport->link_up == v2e->link_status)
return;
vport->link_up = v2e->link_status;
}
Meanwhile, idpf_deinit_task() might have already freed the vport via
idpf_vport_rel() without synchronizing against the mailbox task or clearing
the lookup array first:
drivers/net/ethernet/intel/idpf/idpf_lib.c:idpf_vport_rel() {
kfree(vport);
adapter->num_alloc_vports--;
}
Could this lead to memory corruption if an unsolicited hardware event arrives
while idpf_vc_core_deinit() is tearing down the vports?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012063027.27750-1-luckilystar08@gmail.com?part=2
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-13 6:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 6:30 [PATCH iwl-net v4 0/2] idpf: destroy the vports on shutdown Tian Xun Ng
2026-10-12 6:30 ` [PATCH iwl-net v4 1/2] idpf: don't use the Tx timestamp trackers once removal has started Tian Xun Ng
2026-10-13 6:31 ` sashiko-bot
2026-10-12 6:30 ` [PATCH iwl-net v4 2/2] idpf: keep the mailbox up while tearing down vports on shutdown Tian Xun Ng
2026-10-13 6:31 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox