Re: [PATCH v3 5/7] pcie_sriov: Validate NumVFs

[Michael S. Tsirkin]

On Mon, Feb 12, 2024 at 07:20:33PM +0900, Akihiko Odaki wrote:
> The guest may write NumVFs greater than TotalVFs and that can lead
> to buffer overflow in VF implementations.
> 
> Fixes: 7c0fa8dff811 ("pcie: Add support for Single Root I/O Virtualization 
> (SR/IOV)")
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  hw/pci/pcie_sriov.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> index a1fe65f5d801..da209b7f47fd 100644
> --- a/hw/pci/pcie_sriov.c
> +++ b/hw/pci/pcie_sriov.c
> @@ -176,6 +176,9 @@ static void register_vfs(PCIDevice *dev)
>  
>      assert(sriov_cap > 0);
>      num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
> +    if (num_vfs > pci_get_word(dev->config + sriov_cap + 
> PCI_SRIOV_TOTAL_VF)) {
> +        return;
> +    }

Indeed:
     The results are undefined if NumVFs is set to a value greater than 
TotalVFs.

However I note that hw/nvme/ctrl.c will still poke at NumVFs.

Since it's undefined, I propose a simpler hack and just force it
to PCI_SRIOV_TOTAL_VF. This way everyone can just assume it's ok.


>  
>      dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
>  
> 
> -- 
> 2.43.0