qdev: Use int32_t container for devfn property

Valid range for devfn is -1 to 255 (-1 for automatic assignment). We do
not currently validate this due to devfn being stored as a uint32_t.
This can lead to segfaults and other strange behavior.

We could technically just cast it to int32_t to implement the checking,
but this will not work for visitor-based setting where we may do additional
bounds-checking based on target container type, which is int32_t for this
case.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>

diff --git a/hw/pci.c b/hw/pci.c
index c1ebdde91e9de22823bacf8ec8d96df4aceab678..127b7aca7310b4a9fc77662961f567629ce31d05 100644 (file)
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1537,7 +1537,7 @@ PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
     DeviceState *dev;
 
     dev = qdev_create(&bus->qbus, name);
-    qdev_prop_set_uint32(dev, "addr", devfn);
+    qdev_prop_set_int32(dev, "addr", devfn);
     qdev_prop_set_bit(dev, "multifunction", multifunction);
     return PCI_DEVICE(dev);
 }

diff --git a/hw/pci.h b/hw/pci.h
index c3cacce046ffb3fbd00d889adec66fb748c0622c..7f223c01e10fb76d79e14b6d0e29fbb2740697df 100644 (file)
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -197,7 +197,7 @@ struct PCIDevice {
 
     /* the following fields are read only */
     PCIBus *bus;
-    uint32_t devfn;
+    int32_t devfn;
     char name[64];
     PCIIORegion io_regions[PCI_NUM_REGIONS];
 

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index b7b5597c62499eb822a307f531c9f028b33c2425..d109f89886126fd400722e3a1bb081f205849851 100644 (file)
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -824,7 +824,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque,
 {
     DeviceState *dev = DEVICE(obj);
     Property *prop = opaque;
-    uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
+    int32_t *ptr = qdev_get_prop_ptr(dev, prop);
     unsigned int slot, fn, n;
     Error *local_err = NULL;
     char *str;
@@ -860,7 +860,7 @@ invalid:
 
 static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest, size_t len)
 {
-    uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
+    int32_t *ptr = qdev_get_prop_ptr(dev, prop);
 
     if (*ptr == -1) {
         return snprintf(dest, len, "<unset>");
@@ -875,11 +875,8 @@ PropertyInfo qdev_prop_pci_devfn = {
     .print = print_pci_devfn,
     .get   = get_int32,
     .set   = set_pci_devfn,
-    /* FIXME: this should be -1...255, but the address is stored
-     * into an uint32_t rather than int32_t.
-     */
-    .min   = 0,
-    .max   = 0xFFFFFFFFULL,
+    .min   = -1,
+    .max   = 255,
 };
 
 /* --- blocksize --- */

diff --git a/hw/qdev.h b/hw/qdev.h
index 4e9011947116350de8340c44163e1660612663f9..d07da4576dac55fd4dcb58b1f56a1cf7425130cf 100644 (file)
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -267,7 +267,7 @@ extern PropertyInfo qdev_prop_blocksize;
 #define DEFINE_PROP_HEX64(_n, _s, _f, _d)                       \
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t)
 #define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d)                   \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, uint32_t)
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t)
 
 #define DEFINE_PROP_PTR(_n, _s, _f)             \
     DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, void*)