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)? > if (default_vport) { > for (index = 0; index < adapter->max_vports; index++) { > if (adapter->vports[index]) [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
