Patchwork [v2] pci: Common overflow prevention

login
register
mail settings
Submitter Isaku Yamahata
Date July 29, 2011, 1:01 a.m.
Message ID <20110729010143.GI14976@valinux.co.jp>
Download mbox | patch
Permalink /patch/107316/
State New
Headers show

Comments

Isaku Yamahata - July 29, 2011, 1:01 a.m.
On Thu, Jul 28, 2011 at 11:40:21AM +0300, Michael S. Tsirkin wrote:
> I don't see a problem with this, but could you please clarify when does
> this happen? I think this is only possible for a pci device
> behind an express root. If so, this belongs in pcie_host.c
> 
> I'd also like this info to be recorded in the commit log.

From 1dd598fd35d4e988dc51487829ed66208ca89021 Mon Sep 17 00:00:00 2001
Message-Id: <1dd598fd35d4e988dc51487829ed66208ca89021.1311901239.git.yamahata@valinux.co.jp>
From: Isaku Yamahata <yamahata@valinux.co.jp>
Date: Fri, 29 Jul 2011 09:52:45 +0900
Subject: [PATCH] pcie_host: limit check of pcie_mmcfg_data_write/read

This patch adds the check where the offset in the configuration space
is in its configuration size.

MMCFG area allows access of pcie configuration space to be in
offset [0, 4K).
At the same time, conventional pci devices whose configuration space size
is 256 bytes can be behind pcie-to-pci bridge.
The access whose offset is [256, 4K) should have no effect
on the conventional pci device
Add the limit check and ignore such accesses.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pcie_host.c |   28 ++++++++++++++++++++++------
 1 files changed, 22 insertions(+), 6 deletions(-)
Michael S. Tsirkin - July 29, 2011, 5:03 a.m.
On Fri, Jul 29, 2011 at 10:01:43AM +0900, Isaku Yamahata wrote:
> On Thu, Jul 28, 2011 at 11:40:21AM +0300, Michael S. Tsirkin wrote:
> > I don't see a problem with this, but could you please clarify when does
> > this happen? I think this is only possible for a pci device
> > behind an express root. If so, this belongs in pcie_host.c
> > 
> > I'd also like this info to be recorded in the commit log.
> 
> >From 1dd598fd35d4e988dc51487829ed66208ca89021 Mon Sep 17 00:00:00 2001
> Message-Id: <1dd598fd35d4e988dc51487829ed66208ca89021.1311901239.git.yamahata@valinux.co.jp>
> From: Isaku Yamahata <yamahata@valinux.co.jp>
> Date: Fri, 29 Jul 2011 09:52:45 +0900
> Subject: [PATCH] pcie_host: limit check of pcie_mmcfg_data_write/read
> 
> This patch adds the check where the offset in the configuration space
> is in its configuration size.
> 
> MMCFG area allows access of pcie configuration space to be in
> offset [0, 4K).
> At the same time, conventional pci devices whose configuration space size
> is 256 bytes can be behind pcie-to-pci bridge.
> The access whose offset is [256, 4K) should have no effect
> on the conventional pci device
> Add the limit check and ignore such accesses.
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

I tweaked the commit log and applied this.
Thanks!

Patch

diff --git a/hw/pcie_host.c b/hw/pcie_host.c
index f0b3d13..f9fea3d 100644
--- a/hw/pcie_host.c
+++ b/hw/pcie_host.c
@@ -56,23 +56,39 @@  static void pcie_mmcfg_data_write(PCIBus *s,
                                   uint32_t mmcfg_addr, uint32_t val, int len)
 {
     PCIDevice *pci_dev = pcie_dev_find_by_mmcfg_addr(s, mmcfg_addr);
+    uint32_t addr;
+    uint32_t limit;
 
     if (!pci_dev) {
         return;
     }
-    pci_host_config_write_common(pci_dev, PCIE_MMCFG_CONFOFFSET(mmcfg_addr),
-                                 pci_config_size(pci_dev), val, len);
+    addr = PCIE_MMCFG_CONFOFFSET(mmcfg_addr);
+    limit = pci_config_size(pci_dev);
+    if (limit <= addr) {
+        /* conventional pci device can be behind pcie-to-pci bridge.
+           256 <= addr < 4K has no effects. */
+        return;
+    }
+    pci_host_config_write_common(pci_dev, addr, limit, val, len);
 }
 
-static uint32_t pcie_mmcfg_data_read(PCIBus *s, uint32_t addr, int len)
+static uint32_t pcie_mmcfg_data_read(PCIBus *s, uint32_t mmcfg_addr, int len)
 {
-    PCIDevice *pci_dev = pcie_dev_find_by_mmcfg_addr(s, addr);
+    PCIDevice *pci_dev = pcie_dev_find_by_mmcfg_addr(s, mmcfg_addr);
+    uint32_t addr;
+    uint32_t limit;
 
     if (!pci_dev) {
         return ~0x0;
     }
-    return pci_host_config_read_common(pci_dev, PCIE_MMCFG_CONFOFFSET(addr),
-                                       pci_config_size(pci_dev), len);
+    addr = PCIE_MMCFG_CONFOFFSET(mmcfg_addr);
+    limit = pci_config_size(pci_dev);
+    if (limit <= addr) {
+        /* conventional pci device can be behind pcie-to-pci bridge.
+           256 <= addr < 4K has no effects. */
+        return ~0x0;
+    }
+    return pci_host_config_read_common(pci_dev, addr, limit, len);
 }
 
 static void pcie_mmcfg_data_writeb(void *opaque,