Re: [PATCH net-next] net: mana: Add handler for sriov configure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing `pci_disable_sriov()` in PF driver's `remove` and `shutdown` paths causes resource leaks and potential Use-After-Free.
- [Medium] Missing VF assignment check in `mana_sriov_configure` causes silent failure when disabling SR-IOV.
--

commit 1f4061f22025ea097273f720b3bccac577e7d805
Author: Haiyang Zhang <haiyangz@microsoft.com>

net: mana: Add handler for sriov configure

This commit adds the sriov_configure callback to the mana pci_driver to handle
SR-IOV configuration. It also exports pci_vf_drivers_autoprobe() and disables
VF autoprobe when running as a PF driver on bare metal.

> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 3bc3fff559991..767f11d5b351b 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
[ ... ]
> @@ -2262,6 +2267,20 @@ static void mana_gd_shutdown(struct pci_dev *pdev)
>  	pci_disable_device(pdev);
>  }

Does this code need to explicitly call pci_disable_sriov() during driver
remove and shutdown?

If the PF driver is unbound while VFs are enabled, the PCI core logs a
warning but does not automatically disable SR-IOV. With the VFs remaining
active, could they interact with dead hardware or trigger host-side PF events
that rely on the freed gdma_context, resulting in a use-after-free?

> +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);
> +
> +	return err ? err : numvfs;

If VFs are currently assigned to a guest VM (e.g., via VFIO), the PCI core's
sriov_disable() will internally abort the teardown. Because
pci_disable_sriov() returns void, will this function return 0 and falsely
signal to userspace that the disable operation succeeded?

Should this code check pci_vfs_assigned(pdev) and return -EPERM, or use
the generic pci_sriov_configure_simple() helper instead?

> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508220412.15138-1-haiyangz@linux.microsoft.com?part=1