diff mbox

[1/1] Fix assert when get default PCI address property used by vfio-pci

Message ID 46a04f3ac5b60b26d2eff291e30747cd2c591233.1478712729.git.daniel.oram@gmail.com
State New
Headers show

Commit Message

Daniel Oram Nov. 9, 2016, 6:36 p.m. UTC
Allow the PCIHostDeviceAddress structure to work as the host property in vfio-pci when it has it's default value of all fields set to ~0. In this form the property indicates a non-existant device but given the field bit sizes gets asserted as excess (and invalid) precision overflows the string buffer. The BDF of an invalid device "FFFF:FF:FF.F" is returned instead.

Signed-off-by: Daniel Oram <daniel.oram@gmail.com>
---
 hw/core/qdev-properties.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

Comments

Alex Williamson Nov. 9, 2016, 7:38 p.m. UTC | #1
On Wed,  9 Nov 2016 18:36:20 +0000
Daniel Oram <daniel.oram@gmail.com> wrote:

> Allow the PCIHostDeviceAddress structure to work as the host property in vfio-pci when it has it's default value of all fields set to ~0. In this form the property indicates a non-existant device but given the field bit sizes gets asserted as excess (and invalid) precision overflows the string buffer. The BDF of an invalid device "FFFF:FF:FF.F" is returned instead.
> 

nit, wrap your commit log at ~70 chars

> Signed-off-by: Daniel Oram <daniel.oram@gmail.com>
> ---
>  hw/core/qdev-properties.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 311af6d..ed0d5b0 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -705,13 +705,21 @@ static void get_pci_host_devaddr(Object *obj, Visitor *v, const char *name,
>      DeviceState *dev = DEVICE(obj);
>      Property *prop = opaque;
>      PCIHostDeviceAddress *addr = qdev_get_prop_ptr(dev, prop);
> -    char buffer[] = "xxxx:xx:xx.x";
> +    char buffer[] = "ffff:ff:ff.f";
>      char *p = buffer;
>      int rc = 0;
> -
> -    rc = snprintf(buffer, sizeof(buffer), "%04x:%02x:%02x.%d",
> -                  addr->domain, addr->bus, addr->slot, addr->function);
> -    assert(rc == sizeof(buffer) - 1);
> +    
> +    /*
> +     * Catch "invalid" device reference from vfio-pci and allow the 
> +     * default buffer representing the non-existant device to be used.
> +     */
> +    if (~addr->domain || ~addr->bus || ~addr->slot || ~addr->function) {
> +           
> +        rc = snprintf(buffer, sizeof(buffer), "%04x:%02x:%02x.%0d",
> +                      addr->domain, addr->bus, addr->slot, addr->function);
> +        assert(rc == sizeof(buffer) - 1);
> +    }
> +    
>  
>      visit_type_str(v, name, &p, errp);
>  }

Works for me, note that slot and function are actually 5 bits and 3
bits respectively, so an address ending in "ff.f" can easily be
recognized as invalid to the user, the highest actual address would be
"ffff:ff:1f.7".

Reviewed-by: Alex Williamson <alex.williamson@redhat.com>

Can anyone take this for 2.8?  Thanks,

Alex
diff mbox

Patch

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 311af6d..ed0d5b0 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -705,13 +705,21 @@  static void get_pci_host_devaddr(Object *obj, Visitor *v, const char *name,
     DeviceState *dev = DEVICE(obj);
     Property *prop = opaque;
     PCIHostDeviceAddress *addr = qdev_get_prop_ptr(dev, prop);
-    char buffer[] = "xxxx:xx:xx.x";
+    char buffer[] = "ffff:ff:ff.f";
     char *p = buffer;
     int rc = 0;
-
-    rc = snprintf(buffer, sizeof(buffer), "%04x:%02x:%02x.%d",
-                  addr->domain, addr->bus, addr->slot, addr->function);
-    assert(rc == sizeof(buffer) - 1);
+    
+    /*
+     * Catch "invalid" device reference from vfio-pci and allow the 
+     * default buffer representing the non-existant device to be used.
+     */
+    if (~addr->domain || ~addr->bus || ~addr->slot || ~addr->function) {
+           
+        rc = snprintf(buffer, sizeof(buffer), "%04x:%02x:%02x.%0d",
+                      addr->domain, addr->bus, addr->slot, addr->function);
+        assert(rc == sizeof(buffer) - 1);
+    }
+    
 
     visit_type_str(v, name, &p, errp);
 }