qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>,
	qemu-stable@nongnu.org,
	Corentin BAYET <corentin.bayet@reversetactics.com>,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Subject: [PULL 05/17] pcie_sriov: Fix configuration and state synchronization
Date: Fri, 1 Aug 2025 10:25:12 -0400	[thread overview]
Message-ID: <cad9aa6fbdccd95e56e10cfa57c354a20a333717.1754058276.git.mst@redhat.com> (raw)
In-Reply-To: <cover.1754058276.git.mst@redhat.com>

From: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

Fix issues in PCIe SR-IOV configuration register handling that caused
inconsistent internal state due to improper write mask handling and
incorrect migration behavior.

Two main problems were identified:

1. VF Enable bit write mask handling:
   pcie_sriov_config_write() incorrectly assumed that its val parameter
   was already masked, causing it to ignore the actual write mask.
   This led to the VF Enable bit being processed even when masked,
   resulting in incorrect VF registration/unregistration. It is
   identified as CVE-2025-54567.

2. Migration state inconsistency:
   pcie_sriov_pf_post_load() unconditionally called register_vfs()
   regardless of the VF Enable bit state, creating inconsistent
   internal state when VFs should not be enabled. Additionally,
   it failed to properly update the NumVFs write mask based on
   the current configuration. It is identified as CVE-2025-54566.

Root cause analysis revealed that both functions relied on incorrect
special-case assumptions instead of properly reading and consuming
the actual configuration values. This change introduces a unified
consume_config() function that reads actual configuration values and
synchronize the internal state without special-case assumptions.

The solution only adds register read overhead in non-hot-path code
while ensuring correct SR-IOV state management across configuration
writes and migration scenarios.

Fixes: 5e7dd17e4348 ("pcie_sriov: Remove num_vfs from PCIESriovPF")
Fixes: f9efcd47110d ("pcie_sriov: Register VFs after migration")
Fixes: CVE-2025-54566
Fixes: CVE-2025-54567
Cc: qemu-stable@nongnu.org
Reported-by: Corentin BAYET <corentin.bayet@reversetactics.com>
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Message-Id: <20250727-wmask-v2-1-394910b1c0b6@rsg.ci.i.u-tokyo.ac.jp>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pcie_sriov.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index 3ad18744f4..8a4bf0d6f7 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -64,6 +64,27 @@ static void unregister_vfs(PCIDevice *dev)
     pci_set_word(dev->wmask + dev->exp.sriov_cap + PCI_SRIOV_NUM_VF, 0xffff);
 }
 
+static void consume_config(PCIDevice *dev)
+{
+    uint8_t *cfg = dev->config + dev->exp.sriov_cap;
+
+    if (pci_get_word(cfg + PCI_SRIOV_CTRL) & PCI_SRIOV_CTRL_VFE) {
+        register_vfs(dev);
+    } else {
+        uint8_t *wmask = dev->wmask + dev->exp.sriov_cap;
+        uint16_t num_vfs = pci_get_word(cfg + PCI_SRIOV_NUM_VF);
+        uint16_t wmask_val = PCI_SRIOV_CTRL_MSE | PCI_SRIOV_CTRL_ARI;
+
+        unregister_vfs(dev);
+
+        if (num_vfs <= pci_get_word(cfg + PCI_SRIOV_TOTAL_VF)) {
+            wmask_val |= PCI_SRIOV_CTRL_VFE;
+        }
+
+        pci_set_word(wmask + PCI_SRIOV_CTRL, wmask_val);
+    }
+}
+
 static bool pcie_sriov_pf_init_common(PCIDevice *dev, uint16_t offset,
                                       uint16_t vf_dev_id, uint16_t init_vfs,
                                       uint16_t total_vfs, uint16_t vf_offset,
@@ -416,30 +437,13 @@ void pcie_sriov_config_write(PCIDevice *dev, uint32_t address,
     trace_sriov_config_write(dev->name, PCI_SLOT(dev->devfn),
                              PCI_FUNC(dev->devfn), off, val, len);
 
-    if (range_covers_byte(off, len, PCI_SRIOV_CTRL)) {
-        if (val & PCI_SRIOV_CTRL_VFE) {
-            register_vfs(dev);
-        } else {
-            unregister_vfs(dev);
-        }
-    } else if (range_covers_byte(off, len, PCI_SRIOV_NUM_VF)) {
-        uint8_t *cfg = dev->config + sriov_cap;
-        uint8_t *wmask = dev->wmask + sriov_cap;
-        uint16_t num_vfs = pci_get_word(cfg + PCI_SRIOV_NUM_VF);
-        uint16_t wmask_val = PCI_SRIOV_CTRL_MSE | PCI_SRIOV_CTRL_ARI;
-
-        if (num_vfs <= pci_get_word(cfg + PCI_SRIOV_TOTAL_VF)) {
-            wmask_val |= PCI_SRIOV_CTRL_VFE;
-        }
-
-        pci_set_word(wmask + PCI_SRIOV_CTRL, wmask_val);
-    }
+    consume_config(dev);
 }
 
 void pcie_sriov_pf_post_load(PCIDevice *dev)
 {
     if (dev->exp.sriov_cap) {
-        register_vfs(dev);
+        consume_config(dev);
     }
 }
 
-- 
MST



  parent reply	other threads:[~2025-08-01 14:37 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-01 14:24 [PULL 00/17] virtio,pci,pc: bugfixes Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 01/17] virtio: fix off-by-one and invalid access in virtqueue_ordered_fill Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 02/17] vhost: Do not abort on log-start error Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 03/17] vhost: Do not abort on log-stop error Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 04/17] virtio-net: Fix VLAN filter table reset timing Michael S. Tsirkin
2025-08-01 14:25 ` Michael S. Tsirkin [this message]
2025-08-01 14:25 ` [PULL 06/17] hw/i386/amd_iommu: Fix MMIO register write tracing Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 07/17] hw/i386/amd_iommu: Remove unused and wrongly set ats_enabled field Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 08/17] hw/i386/amd_iommu: Move IOAPIC memory region initialization to the end Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 09/17] hw/i386/amd_iommu: Fix amdvi_write*() Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 10/17] hw/i386/amd_iommu: Support MMIO writes to the status register Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 11/17] hw/i386/amd_iommu: Fix event log generation Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 12/17] tests/acpi: virt: add an empty HEST file Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 13/17] tests/qtest/bios-tables-test: extend to also check HEST table Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 14/17] tests/acpi: virt: update HEST file with its current data Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 15/17] intel_iommu: Allow both Status Write and Interrupt Flag in QI wait Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 16/17] MAINTAINERS: add net/vhost* files under `vhost` Michael S. Tsirkin
2025-08-01 14:25 ` [PULL 17/17] net/vdpa: fix potential fd leak in net_init_vhost_vdpa() Michael S. Tsirkin
2025-08-01 19:34 ` [PULL 00/17] virtio,pci,pc: bugfixes Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cad9aa6fbdccd95e56e10cfa57c354a20a333717.1754058276.git.mst@redhat.com \
    --to=mst@redhat.com \
    --cc=corentin.bayet@reversetactics.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=odaki@rsg.ci.i.u-tokyo.ac.jp \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).