On 2024/02/13 19:59, Michael S. Tsirkin wrote:
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.od...@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.
It is still not OK to poke at NumVFs as the guest may set a different
number anytime though it's undefined if NumVFs is set while VFs are
enabled. I think hw/nvme/ctrl.c should be changed to look at
exp.sriov_pf.num_vfs, which holds the committed NumVFs value.
