Patchwork pci-devfn: check that device/slot number is within range

login
register
mail settings
Submitter Don Dutile
Date Sept. 21, 2011, 7:25 p.m.
Message ID <1316633111-32404-1-git-send-email-ddutile@redhat.com>
Download mbox | patch
Permalink /patch/115843/
State New
Headers show

Comments

Don Dutile - Sept. 21, 2011, 7:25 p.m.
Need to check that guest slot/device number is not > 31 or walk off
the devfn table when checking if a devfn is available or not in a guest.

before this fix, passing in an addr=abc  or addr=34,
can crash qemu, sometimes fail gracefully if data past end
of devfn table fails the availability test.

with this fix, get clean error:
Property 'pci-assign.addr' doesn't take value '34'

also tested when no addr= param passed for guest (pcicfg) address,
and that worked as well.

Signed-off-by: Don Dutile <ddutile@redhat.com>

---
 hw/qdev-properties.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Markus Armbruster - Sept. 22, 2011, 8:08 a.m.
Donald Dutile <ddutile@redhat.com> writes:

> Need to check that guest slot/device number is not > 31 or walk off
> the devfn table when checking if a devfn is available or not in a guest.

Here in do_pci_register_device():

    } else if (bus->devices[devfn]) {
        error_report("PCI: devfn %d not available for %s, in use by %s",
                     devfn, name, bus->devices[devfn]->name);
        return NULL;
    }

> before this fix, passing in an addr=abc  or addr=34,
> can crash qemu, sometimes fail gracefully if data past end
> of devfn table fails the availability test.
>
> with this fix, get clean error:
> Property 'pci-assign.addr' doesn't take value '34'
>
> also tested when no addr= param passed for guest (pcicfg) address,
> and that worked as well.
>
> Signed-off-by: Don Dutile <ddutile@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Anthony Liguori - Sept. 23, 2011, 4:06 p.m.
On 09/21/2011 02:25 PM, Donald Dutile wrote:
> Need to check that guest slot/device number is not>  31 or walk off
> the devfn table when checking if a devfn is available or not in a guest.
>
> before this fix, passing in an addr=abc  or addr=34,
> can crash qemu, sometimes fail gracefully if data past end
> of devfn table fails the availability test.
>
> with this fix, get clean error:
> Property 'pci-assign.addr' doesn't take value '34'
>
> also tested when no addr= param passed for guest (pcicfg) address,
> and that worked as well.
>
> Signed-off-by: Don Dutile<ddutile@redhat.com>

Applied.  Thanks.

Regards,

Anthony Liguori

>
> ---
>   hw/qdev-properties.c |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 7ce95b6..e0e54aa 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -524,6 +524,8 @@ static int parse_pci_devfn(DeviceState *dev, Property *prop, const char *str)
>           return -EINVAL;
>       if (fn>  7)
>           return -EINVAL;
> +    if (slot>  31)
> +        return -EINVAL;
>       *ptr = slot<<  3 | fn;
>       return 0;
>   }

Patch

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 7ce95b6..e0e54aa 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -524,6 +524,8 @@  static int parse_pci_devfn(DeviceState *dev, Property *prop, const char *str)
         return -EINVAL;
     if (fn > 7)
         return -EINVAL;
+    if (slot > 31)
+        return -EINVAL;
     *ptr = slot << 3 | fn;
     return 0;
 }