1. Patchew
  2. QEMU
  3. View series

[PATCH] pcie_sriov: fix wrong write width and off-by-one in pf_reset

lirongqing posted 1 patch 1 week, 2 days ago
There is a newer version of this series
hw/pci/pcie_sriov.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
[PATCH] pcie_sriov: fix wrong write width and off-by-one in pf_reset
Posted by lirongqing 1 week, 2 days ago 
From: Li RongQing <lirongqing@baidu.com>

pcie_sriov_pf_reset() restores the VF BAR type registers using
pci_set_quad() with a 4-byte stride. Since each VF BAR register is
4 bytes wide, the 8-byte write also covers the following BAR
register. Although subsequent iterations overwrite most of the
unintended writes, the access is incorrect and the final iteration
can write beyond the VF BAR registers.

SR-IOV defines six VF BAR registers and does not provide a VF ROM BAR.
However, several SR-IOV code paths use PCI_NUM_REGIONS, which includes
the ROM slot and therefore has a value of seven.

In particular, the final iteration in pcie_sriov_pf_reset() currently
writes a quadword at the offset of the seventh region. This starts at
the byte immediately following the six VF BAR registers and can
overwrite subsequent fields in the SR-IOV capability.

Use pci_set_long() to match the 4-byte VF BAR register width, and use
PCI_SRIOV_NUM_BARS when iterating over SR-IOV VF BARs. This also makes
the BAR count consistent with the assertion in
pcie_sriov_pf_init_vf_bar().

Fixes: 7c0fa8dff811b5 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)")
Fixes: c8bc4db403e176 ("pcie_sriov: Reset SR-IOV extended capability")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 hw/pci/pcie_sriov.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index c41ac95..6122a26 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -229,7 +229,7 @@ void pcie_sriov_pf_init_vf_bar(PCIDevice *dev, int region_num,
 
     assert(sriov_cap > 0);
     assert(region_num >= 0);
-    assert(region_num < PCI_NUM_REGIONS);
+    assert(region_num < PCI_SRIOV_NUM_BARS);
     assert(region_num != PCI_ROM_SLOT);
 
     wmask = ~(size - 1);
@@ -308,7 +308,7 @@ int16_t pcie_sriov_pf_init_from_user_created_vfs(PCIDevice *dev,
             return -1;
         }
 
-        for (size_t j = 0; j < PCI_NUM_REGIONS; j++) {
+        for (size_t j = 0; j < PCI_SRIOV_NUM_BARS; j++) {
             if (vfs[i]->io_regions[j].size != vfs[0]->io_regions[j].size ||
                 vfs[i]->io_regions[j].type != vfs[0]->io_regions[j].type) {
                 error_setg(errp, "inconsistent SR-IOV BARs");
@@ -344,7 +344,7 @@ int16_t pcie_sriov_pf_init_from_user_created_vfs(PCIDevice *dev,
     dev->exp.sriov_pf.vf = vfs;
     dev->exp.sriov_pf.vf_user_created = true;
 
-    for (i = 0; i < PCI_NUM_REGIONS; i++) {
+    for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
         PCIIORegion *region = &vfs[0]->io_regions[i];
 
         if (region->size) {
@@ -463,8 +463,8 @@ void pcie_sriov_pf_reset(PCIDevice *dev)
      */
     pci_set_word(dev->config + sriov_cap + PCI_SRIOV_SYS_PGSIZE, 0x1);
 
-    for (uint16_t i = 0; i < PCI_NUM_REGIONS; i++) {
-        pci_set_quad(dev->config + sriov_cap + PCI_SRIOV_BAR + i * 4,
+    for (uint16_t i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
+        pci_set_long(dev->config + sriov_cap + PCI_SRIOV_BAR + i * 4,
                      dev->exp.sriov_pf.vf_bar_type[i]);
     }
 }
-- 
2.9.4
Re: [PATCH] pcie_sriov: fix wrong write width and off-by-one in pf_reset
Posted by Michael S. Tsirkin 1 week, 2 days ago 
On Thu, Sep 17, 2026 at 01:38:18PM +0800, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> pcie_sriov_pf_reset() restores the VF BAR type registers using
> pci_set_quad() with a 4-byte stride. Since each VF BAR register is
> 4 bytes wide, the 8-byte write also covers the following BAR
> register. Although subsequent iterations overwrite most of the
> unintended writes, the access is incorrect and the final iteration
> can write beyond the VF BAR registers.
> 
> SR-IOV defines six VF BAR registers and does not provide a VF ROM BAR.
> However, several SR-IOV code paths use PCI_NUM_REGIONS, which includes
> the ROM slot and therefore has a value of seven.
> 
> In particular, the final iteration in pcie_sriov_pf_reset() currently
> writes a quadword at the offset of the seventh region. This starts at
> the byte immediately following the six VF BAR registers and can
> overwrite subsequent fields in the SR-IOV capability.
> 
> Use pci_set_long() to match the 4-byte VF BAR register width, and use
> PCI_SRIOV_NUM_BARS when iterating over SR-IOV VF BARs. This also makes
> the BAR count consistent with the assertion in
> pcie_sriov_pf_init_vf_bar().
> 
> Fixes: 7c0fa8dff811b5 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)")

i do not see the bug in that version.
do u mean 19e55471d4e8 maybe?

> Fixes: c8bc4db403e176 ("pcie_sriov: Reset SR-IOV extended capability")
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  hw/pci/pcie_sriov.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> index c41ac95..6122a26 100644
> --- a/hw/pci/pcie_sriov.c
> +++ b/hw/pci/pcie_sriov.c
> @@ -229,7 +229,7 @@ void pcie_sriov_pf_init_vf_bar(PCIDevice *dev, int region_num,
>  
>      assert(sriov_cap > 0);
>      assert(region_num >= 0);
> -    assert(region_num < PCI_NUM_REGIONS);
> +    assert(region_num < PCI_SRIOV_NUM_BARS);
>      assert(region_num != PCI_ROM_SLOT);
>  
>      wmask = ~(size - 1);
> @@ -308,7 +308,7 @@ int16_t pcie_sriov_pf_init_from_user_created_vfs(PCIDevice *dev,
>              return -1;
>          }
>  
> -        for (size_t j = 0; j < PCI_NUM_REGIONS; j++) {
> +        for (size_t j = 0; j < PCI_SRIOV_NUM_BARS; j++) {
>              if (vfs[i]->io_regions[j].size != vfs[0]->io_regions[j].size ||
>                  vfs[i]->io_regions[j].type != vfs[0]->io_regions[j].type) {
>                  error_setg(errp, "inconsistent SR-IOV BARs");
> @@ -344,7 +344,7 @@ int16_t pcie_sriov_pf_init_from_user_created_vfs(PCIDevice *dev,
>      dev->exp.sriov_pf.vf = vfs;
>      dev->exp.sriov_pf.vf_user_created = true;
>  
> -    for (i = 0; i < PCI_NUM_REGIONS; i++) {
> +    for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
>          PCIIORegion *region = &vfs[0]->io_regions[i];
>  
>          if (region->size) {
> @@ -463,8 +463,8 @@ void pcie_sriov_pf_reset(PCIDevice *dev)
>       */
>      pci_set_word(dev->config + sriov_cap + PCI_SRIOV_SYS_PGSIZE, 0x1);
>  
> -    for (uint16_t i = 0; i < PCI_NUM_REGIONS; i++) {
> -        pci_set_quad(dev->config + sriov_cap + PCI_SRIOV_BAR + i * 4,
> +    for (uint16_t i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> +        pci_set_long(dev->config + sriov_cap + PCI_SRIOV_BAR + i * 4,
>                       dev->exp.sriov_pf.vf_bar_type[i]);
>      }
>  }
> -- 
> 2.9.4
答复: [????] Re: [PATCH] pcie_sriov: fix wrong write width and off-by-one in pf_reset
Posted by Li,Rongqing 1 week, 1 day ago 
> On Thu, Sep 17, 2026 at 01:38:18PM +0800, lirongqing wrote:
> > From: Li RongQing <lirongqing@baidu.com>
> >
> > pcie_sriov_pf_reset() restores the VF BAR type registers using
> > pci_set_quad() with a 4-byte stride. Since each VF BAR register is
> > 4 bytes wide, the 8-byte write also covers the following BAR register.
> > Although subsequent iterations overwrite most of the unintended
> > writes, the access is incorrect and the final iteration can write
> > beyond the VF BAR registers.
> >
> > SR-IOV defines six VF BAR registers and does not provide a VF ROM BAR.
> > However, several SR-IOV code paths use PCI_NUM_REGIONS, which includes
> > the ROM slot and therefore has a value of seven.
> >
> > In particular, the final iteration in pcie_sriov_pf_reset() currently
> > writes a quadword at the offset of the seventh region. This starts at
> > the byte immediately following the six VF BAR registers and can
> > overwrite subsequent fields in the SR-IOV capability.
> >
> > Use pci_set_long() to match the 4-byte VF BAR register width, and use
> > PCI_SRIOV_NUM_BARS when iterating over SR-IOV VF BARs. This also makes
> > the BAR count consistent with the assertion in
> > pcie_sriov_pf_init_vf_bar().
> >
> > Fixes: 7c0fa8dff811b5 ("pcie: Add support for Single Root I/O
> > Virtualization (SR/IOV)")
> 
> i do not see the bug in that version.
> do u mean 19e55471d4e8 maybe?
> 

You are right. Thanks for catching that! The issue was indeed introduced by commit 19e55471d4e8, not the initial SR/IOV support commit.

thanks

[Li,Rongqing] 


> > Fixes: c8bc4db403e176 ("pcie_sriov: Reset SR-IOV extended capability")
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> >  hw/pci/pcie_sriov.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c index
> > c41ac95..6122a26 100644
> > --- a/hw/pci/pcie_sriov.c
> > +++ b/hw/pci/pcie_sriov.c
> > @@ -229,7 +229,7 @@ void pcie_sriov_pf_init_vf_bar(PCIDevice *dev, int
> > region_num,
> >
> >      assert(sriov_cap > 0);
> >      assert(region_num >= 0);
> > -    assert(region_num < PCI_NUM_REGIONS);
> > +    assert(region_num < PCI_SRIOV_NUM_BARS);
> >      assert(region_num != PCI_ROM_SLOT);
> >
> >      wmask = ~(size - 1);
> > @@ -308,7 +308,7 @@ int16_t
> pcie_sriov_pf_init_from_user_created_vfs(PCIDevice *dev,
> >              return -1;
> >          }
> >
> > -        for (size_t j = 0; j < PCI_NUM_REGIONS; j++) {
> > +        for (size_t j = 0; j < PCI_SRIOV_NUM_BARS; j++) {
> >              if (vfs[i]->io_regions[j].size != vfs[0]->io_regions[j].size ||
> >                  vfs[i]->io_regions[j].type != vfs[0]->io_regions[j].type) {
> >                  error_setg(errp, "inconsistent SR-IOV BARs"); @@
> > -344,7 +344,7 @@ int16_t
> pcie_sriov_pf_init_from_user_created_vfs(PCIDevice *dev,
> >      dev->exp.sriov_pf.vf = vfs;
> >      dev->exp.sriov_pf.vf_user_created = true;
> >
> > -    for (i = 0; i < PCI_NUM_REGIONS; i++) {
> > +    for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> >          PCIIORegion *region = &vfs[0]->io_regions[i];
> >
> >          if (region->size) {
> > @@ -463,8 +463,8 @@ void pcie_sriov_pf_reset(PCIDevice *dev)
> >       */
> >      pci_set_word(dev->config + sriov_cap + PCI_SRIOV_SYS_PGSIZE,
> > 0x1);
> >
> > -    for (uint16_t i = 0; i < PCI_NUM_REGIONS; i++) {
> > -        pci_set_quad(dev->config + sriov_cap + PCI_SRIOV_BAR + i * 4,
> > +    for (uint16_t i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> > +        pci_set_long(dev->config + sriov_cap + PCI_SRIOV_BAR + i * 4,
> >                       dev->exp.sriov_pf.vf_bar_type[i]);
> >      }
> >  }
> > --
> > 2.9.4

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.