Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Disabling SR-IOV without checking if Virtual Functions (VFs) are assigned to a guest VM. - [High] Missing SR-IOV cleanup during PF driver removal and shutdown. --
commit 3df823cc57367f69930deb6dc48172ce7d77e785 Author: Haiyang Zhang <[email protected]> net: mana: Add handler for sriov configure Add callback function for the pci_driver / sriov_configure. It asks the NIC to provide certain number of VFs, or disable VFs if the request is zero. > diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c > b/drivers/net/ethernet/microsoft/mana/gdma_main.c > index e8b7ffb47eb98..f7a79874fd0eb 100644 > --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c > +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c > @@ -2496,6 +2496,20 @@ static void mana_gd_shutdown(struct pci_dev *pdev) > pci_disable_device(pdev); > } [Severity: High] With the addition of SR-IOV support in this patch, do mana_gd_shutdown() and mana_gd_remove() need to call pci_disable_sriov()? If a user enables VFs via sysfs and then unbinds the driver or shuts down the system, could this omission introduce a regression where orphaned, dead VF PCI devices are left in the kernel? Accessing these orphaned VFs might trigger use-after-free crashes or hardware faults, as the underlying PF hardware and driver context have been destroyed. > +static int mana_sriov_configure(struct pci_dev *pdev, int numvfs) > +{ > + int err = 0; > + > + dev_info(&pdev->dev, "Requested num VFs: %d\n", numvfs); > + > + if (numvfs > 0) > + err = pci_enable_sriov(pdev, numvfs); > + else > + pci_disable_sriov(pdev); [Severity: High] Should this path check if the VFs are currently assigned to a guest VM before disabling SR-IOV? If a privileged user writes 0 to sriov_numvfs while VFs are actively assigned to a VM (e.g., via vfio-pci), could forcefully removing the PCI devices cause a regression resulting in host IOMMU faults and guest kernel panics? Would it be better to check pci_vfs_assigned(pdev) here, or use the pci_sriov_configure_simple() helper to safely enforce this? > + > + return err ? err : numvfs; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
