On 10/13/2026 9:41 AM, Tantilov, Emil S wrote:
On 10/9/2026 2:56 PM, [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
- [High] idpf: double free of adapter->ptp due to concurrent
idpf_ptp_release()
--
commit 140d271eb2e765967ee1a351714e7ad1ecc7d8f6
Author: Emil Tantilov <[email protected]>
idpf: release PTP before freeing vports on the init task error path
Release PTP, which cancels the aux worker, before the unwind frees the
vports, matching the idpf_vc_core_deinit() order. Previously it was
possible for idpf_ptp_update_cached_phctime() which runs via the PTP
auxiliary worker to read a non-NULL but already freed slot, pass its NULL
check, and dereference the freed vport.
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c b/drivers/
net/ethernet/intel/idpf/idpf_lib.c
index a1c739cdd5406..9c541dd6ed4d4 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -1747,6 +1747,8 @@ void idpf_init_task(struct work_struct *work)
return;
unwind_vports:
+ idpf_ptp_release(adapter);
+
[Severity: High]
Does this create a race condition that can result in a double free of
adapter->ptp?
If idpf_remove() or a hard reset is triggered, it invokes
idpf_vc_core_deinit().
idpf_vc_core_deinit() calls idpf_ptp_release() before it calls
idpf_deinit_task(), which is what cancels this init_task:
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c:idpf_vc_core_deinit() {
...
idpf_ptp_release(adapter);
idpf_deinit_task(adapter);
...
}
Concurrently, if this asynchronous idpf_init_task() fails during
initialization, its error path (unwind_vports) also calls
idpf_ptp_release()
without locking or serialization against concurrent teardown.
idpf_ptp_release() performs an unlocked check-and-free:
drivers/net/ethernet/intel/idpf/idpf_ptp.c:idpf_ptp_release() {
struct idpf_ptp *ptp = adapter->ptp;
if (!ptp)
return;
...
kfree(ptp);
adapter->ptp = NULL;
}
If both threads execute this concurrently, could both read the non-
NULL pointer,
leading to concurrent executions of ptp_clock_unregister() and a
double free
via kfree(ptp)?
Actually, idpf_init_task() cannot run concurrently with
idpf_vc_core_deinit().
The init task is only queued from idpf_vc_core_init(), and from itself
to create the next vport. idpf_vc_core_init() is only called from
idpf_init_hard_reset(), which runs from the event task, on driver load
as well as on a reset. Once idpf_vc_core_init() succeeds,
idpf_init_hard_reset() waits for the init task to finish:
while (test_bit(IDPF_HR_RESET_IN_PROG, adapter->flags))
msleep(100);
The init task clears IDPF_HR_RESET_IN_PROG only in its last run, at the
end of the success path, or as the last statement of the unwind, after
its idpf_ptp_release(). The bit stays set while it re-queues itself.
idpf_remove() and idpf_shutdown() cancel the event task before calling
idpf_vc_core_deinit(), and a hard reset calls it from the event task
itself, whose previous run has already waited for the init task. So
idpf_vc_core_deinit() finds either adapter->ptp intact, or already
released and cleared by the unwind, in which case idpf_ptp_release()
returns early. This patch does not change that: the unwind already
released PTP before clearing IDPF_HR_RESET_IN_PROG, it now does so
before freeing the vports as well.
Two concurrent releases are possible on shutdown, but between two
idpf_vc_core_deinit() calls: idpf_shutdown() does not set
IDPF_REMOVE_IN_PROG, so a Tx timeout or an RDMA reset request can
re-queue the event task after it was cancelled and start a hard reset
alongside the shutdown. That is being addressed by different patch,
pending on -net:
https://lore.kernel.org/intel-wired-lan/[email protected]/T/#u
which sets the bit in idpf_shutdown() as in idpf_remove().
if (default_vport) {
for (index = 0; index < adapter->max_vports; index++) {
if (adapter->vports[index])
[ ... ]
I will address it in v2.
Correction, I think this series can stay as is with the one hit above
being dealt with in a patch in the "destroy vport on shutdown" series:
https://lore.kernel.org/intel-wired-lan/[email protected]/T/#t
Sorry for the confusion,
Emil
