[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [PATCH 3/9] pci: Make bounds checks on config space acces

From: Alexander Graf
Subject: Re: [Qemu-ppc] [PATCH 3/9] pci: Make bounds checks on config space accesses actually work
Date: Thu, 12 Jan 2012 14:19:59 +0100
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20110221 SUSE/3.1.8 Thunderbird/3.1.8

On 01/12/2012 06:46 AM, David Gibson wrote:
The pci_host_config_{read,write}_common() functions perform PCI config
accesses.  They take a limit parameter which they appear to be supposed
to bounds check against, however the bounds checking logic, such as it is,
is completely broken.

Currently, it takes the minimum of the supplied length and the remaining
space in the region and passes that as the length to the underlying
config_{read,write} function pointer.  This means that accesses which
partially overrun the region will be silently truncated - which makes
little sense.  Accesses which entirely overrun the region will *not*
be blocked (an exploitable bug), because in that case (limit - addr) will
be negative and so the unsigned MIN will always return len instead.  Even
if signed arithmetic was used, the config_{read,write} callback wouldn't
know what to do with a negative len parameter.

This patch handles things more sanely by simply ignoring writes which
overrun, and returning -1 for reads, which is the usual hardware convention
for reads to unpopulated IO regions.

Signed-off-by: David Gibson<address@hidden>

Michael, please ack or apply yourself. I'll cache this in my tree regardless so it doesn't get lost.


  hw/pci_host.c |   10 ++++++++--
  1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/pci_host.c b/hw/pci_host.c
index 44c6c20..16b3ac3 100644
--- a/hw/pci_host.c
+++ b/hw/pci_host.c
@@ -51,14 +51,20 @@ void pci_host_config_write_common(PCIDevice *pci_dev, 
uint32_t addr,
                                    uint32_t limit, uint32_t val, uint32_t len)
      assert(len<= 4);
-    pci_dev->config_write(pci_dev, addr, val, MIN(len, limit - addr));
+    if ((addr + len)<= limit) {
+        pci_dev->config_write(pci_dev, addr, val, len);
+    }

  uint32_t pci_host_config_read_common(PCIDevice *pci_dev, uint32_t addr,
                                       uint32_t limit, uint32_t len)
      assert(len<= 4);
-    return pci_dev->config_read(pci_dev, addr, MIN(len, limit - addr));
+    if ((addr + len)<= limit) {
+        return pci_dev->config_read(pci_dev, addr, len);
+    } else {
+        return ~0x0;
+    }

  void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len)

reply via email to

[Prev in Thread] Current Thread [Next in Thread]