Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] net: idpf: use-after-free of PTP tx_tstamp_caps during shutdown --
commit 5449e12bf292e1f9d4473ed0d51fe60b2a67f6b3 Author: Tian Xun Ng <[email protected]> idpf: keep the mailbox up while tearing down vports on shutdown 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. > diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c > b/drivers/net/ethernet/intel/idpf/idpf_main.c > index 129bccaa6baa8..ec751f4a6deb6 100644 > --- a/drivers/net/ethernet/intel/idpf/idpf_main.c > +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c > @@ -194,8 +194,17 @@ 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); [Severity: High] Does setting IDPF_REMOVE_IN_PROG here expose a use-after-free race during reboot? When this flag is set, idpf_vc_core_deinit() defers shutting down the transaction manager (via libie_ctlq_xn_shutdown()) until the very end. However, it also calls idpf_ptp_release() earlier in the function, which synchronously frees vport->tx_tstamp_caps via idpf_ptp_release_vport_tstamp() in drivers/net/ethernet/intel/idpf/idpf_ptp.c. Because the transaction manager and mailbox task remain active during this window, an asynchronous PTP Tx timestamp reply can still be received. If that happens, the mailbox handler idpf_ptp_get_tx_tstamp_async_handler() in drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c can dereference the freed tx_tstamp_caps pointer, leading to a panic. While the commit message mentions that a race already exists in the remove path, setting this flag appears to newly expose the same vulnerability in the shutdown path. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
