[libvirt] [PATCH] conf: validate that PCI controller index is < 256

Laine Stump posted 1 patch 7 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170327004928.9874-1-laine@laine.org
src/conf/domain_conf.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
[libvirt] [PATCH] conf: validate that PCI controller index is < 256
Posted by Laine Stump 7 years ago
This is the maximum for many reasons, for starters because index ==
bus number, and a controller's bus number is 8 bits.

This incidentally resolves: https://bugzilla.redhat.com/1329090
---
 src/conf/domain_conf.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6bbc6a2..2139ab0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4846,6 +4846,24 @@ virDomainNetDefValidate(const virDomainNetDef *net)
 
 
 static int
+virDomainControllerDefValidate(const virDomainControllerDef *controller)
+{
+    if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
+        if (controller->idx > 255) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("PCI controller index %d too high, or too many "
+                             "PCI controllers. A maximum of 256 PCI "
+                             "controllers is allowed, and the maximum value "
+                             "of the index attribute is 255"),
+                           controller->idx);
+            return -1;
+        }
+    }
+    return 0;
+}
+
+
+static int
 virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
                                    const virDomainDef *def)
 {
@@ -4867,6 +4885,7 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
     case VIR_DOMAIN_DEVICE_HOSTDEV:
     case VIR_DOMAIN_DEVICE_WATCHDOG:
     case VIR_DOMAIN_DEVICE_CONTROLLER:
+        return virDomainControllerDefValidate(dev->data.controller);
     case VIR_DOMAIN_DEVICE_GRAPHICS:
     case VIR_DOMAIN_DEVICE_HUB:
     case VIR_DOMAIN_DEVICE_SMARTCARD:
-- 
2.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: validate that PCI controller index is < 256
Posted by Martin Kletzander 7 years ago
On Sun, Mar 26, 2017 at 08:49:28PM -0400, Laine Stump wrote:
>This is the maximum for many reasons, for starters because index ==
>bus number, and a controller's bus number is 8 bits.
>
>This incidentally resolves: https://bugzilla.redhat.com/1329090
>---
> src/conf/domain_conf.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 6bbc6a2..2139ab0 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -4846,6 +4846,24 @@ virDomainNetDefValidate(const virDomainNetDef *net)
>
>
> static int
>+virDomainControllerDefValidate(const virDomainControllerDef *controller)
>+{
>+    if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
>+        if (controller->idx > 255) {
>+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                           _("PCI controller index %d too high, or too many "

The first line might be enough, maybe adding the max value there.

>+                             "PCI controllers. A maximum of 256 PCI "
>+                             "controllers is allowed, and the maximum value "
>+                             "of the index attribute is 255"),
>+                           controller->idx);
>+            return -1;
>+        }
>+    }
>+    return 0;
>+}
>+
>+
>+static int
> virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
>                                    const virDomainDef *def)
> {
>@@ -4867,6 +4885,7 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
>     case VIR_DOMAIN_DEVICE_HOSTDEV:
>     case VIR_DOMAIN_DEVICE_WATCHDOG:
>     case VIR_DOMAIN_DEVICE_CONTROLLER:
>+        return virDomainControllerDefValidate(dev->data.controller);

Do you really mean to fall-through from hostdev, watchdog and so on? ;)

>     case VIR_DOMAIN_DEVICE_GRAPHICS:
>     case VIR_DOMAIN_DEVICE_HUB:
>     case VIR_DOMAIN_DEVICE_SMARTCARD:
>--
>2.9.3
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: validate that PCI controller index is < 256
Posted by Laine Stump 7 years ago
On 03/27/2017 03:41 AM, Martin Kletzander wrote:
> On Sun, Mar 26, 2017 at 08:49:28PM -0400, Laine Stump wrote:
>> This is the maximum for many reasons, for starters because index ==
>> bus number, and a controller's bus number is 8 bits.
>>
>> This incidentally resolves: https://bugzilla.redhat.com/1329090
>> ---
>> src/conf/domain_conf.c | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 6bbc6a2..2139ab0 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -4846,6 +4846,24 @@ virDomainNetDefValidate(const virDomainNetDef
>> *net)
>>
>>
>> static int
>> +virDomainControllerDefValidate(const virDomainControllerDef *controller)
>> +{
>> +    if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
>> +        if (controller->idx > 255) {
>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                           _("PCI controller index %d too high, or
>> too many "
> 
> The first line might be enough, maybe adding the max value there.

Okay, I adjusted it.

> 
>> +                             "PCI controllers. A maximum of 256 PCI "
>> +                             "controllers is allowed, and the maximum
>> value "
>> +                             "of the index attribute is 255"),
>> +                           controller->idx);
>> +            return -1;
>> +        }
>> +    }
>> +    return 0;
>> +}
>> +
>> +
>> +static int
>> virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
>>                                    const virDomainDef *def)
>> {
>> @@ -4867,6 +4885,7 @@ virDomainDeviceDefValidateInternal(const
>> virDomainDeviceDef *dev,
>>     case VIR_DOMAIN_DEVICE_HOSTDEV:
>>     case VIR_DOMAIN_DEVICE_WATCHDOG:
>>     case VIR_DOMAIN_DEVICE_CONTROLLER:
>> +        return virDomainControllerDefValidate(dev->data.controller);
> 
> Do you really mean to fall-through from hostdev, watchdog and so on? ;)

No, it was just late in the day, I was on a roll and didn't pay
attention. I moved this up above the other NOP cases. I'm just attaching
the patch here. Is that good enough for an ACK, or do you want a full
re-send?


> 
>>     case VIR_DOMAIN_DEVICE_GRAPHICS:
>>     case VIR_DOMAIN_DEVICE_HUB:
>>     case VIR_DOMAIN_DEVICE_SMARTCARD:
>> -- 
>> 2.9.3
>>
>> -- 
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: validate that PCI controller index is < 256
Posted by Martin Kletzander 7 years ago
On Mon, Mar 27, 2017 at 09:51:28AM -0400, Laine Stump wrote:
>On 03/27/2017 03:41 AM, Martin Kletzander wrote:
>> On Sun, Mar 26, 2017 at 08:49:28PM -0400, Laine Stump wrote:
>>> This is the maximum for many reasons, for starters because index ==
>>> bus number, and a controller's bus number is 8 bits.
>>>
>>> This incidentally resolves: https://bugzilla.redhat.com/1329090
>>> ---
>>> src/conf/domain_conf.c | 19 +++++++++++++++++++
>>> 1 file changed, 19 insertions(+)
>>>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index 6bbc6a2..2139ab0 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -4846,6 +4846,24 @@ virDomainNetDefValidate(const virDomainNetDef
>>> *net)
>>>
>>>
>>> static int
>>> +virDomainControllerDefValidate(const virDomainControllerDef *controller)
>>> +{
>>> +    if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
>>> +        if (controller->idx > 255) {
>>> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>> +                           _("PCI controller index %d too high, or
>>> too many "
>>
>> The first line might be enough, maybe adding the max value there.
>
>Okay, I adjusted it.
>
>>
>>> +                             "PCI controllers. A maximum of 256 PCI "
>>> +                             "controllers is allowed, and the maximum
>>> value "
>>> +                             "of the index attribute is 255"),
>>> +                           controller->idx);
>>> +            return -1;
>>> +        }
>>> +    }
>>> +    return 0;
>>> +}
>>> +
>>> +
>>> +static int
>>> virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
>>>                                    const virDomainDef *def)
>>> {
>>> @@ -4867,6 +4885,7 @@ virDomainDeviceDefValidateInternal(const
>>> virDomainDeviceDef *dev,
>>>     case VIR_DOMAIN_DEVICE_HOSTDEV:
>>>     case VIR_DOMAIN_DEVICE_WATCHDOG:
>>>     case VIR_DOMAIN_DEVICE_CONTROLLER:
>>> +        return virDomainControllerDefValidate(dev->data.controller);
>>
>> Do you really mean to fall-through from hostdev, watchdog and so on? ;)
>
>No, it was just late in the day, I was on a roll and didn't pay
>attention. I moved this up above the other NOP cases. I'm just attaching
>the patch here. Is that good enough for an ACK, or do you want a full
>re-send?
>

Well, you resent the whole patch ;)  Of course that's fine, ACK.

>
>>
>>>     case VIR_DOMAIN_DEVICE_GRAPHICS:
>>>     case VIR_DOMAIN_DEVICE_HUB:
>>>     case VIR_DOMAIN_DEVICE_SMARTCARD:
>>> --
>>> 2.9.3
>>>
>>> --
>>> libvir-list mailing list
>>> libvir-list@redhat.com
>>> https://www.redhat.com/mailman/listinfo/libvir-list
>

>From 4416cde9d9ce36581b56a3de905122c54c2cfb0e Mon Sep 17 00:00:00 2001
>From: Laine Stump <laine@laine.org>
>Date: Sun, 26 Mar 2017 20:47:17 -0400
>Subject: [PATCH] conf: validate that PCI controller index is < 256
>
>This is the maximum for many reasons, for starters because index ==
>bus number, and a controller's bus number is 8 bits.
>
>This incidentally resolves: https://bugzilla.redhat.com/1329090
>---
> src/conf/domain_conf.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 6bbc6a2..b463f3b 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -4846,6 +4846,21 @@ virDomainNetDefValidate(const virDomainNetDef *net)
>
>
> static int
>+virDomainControllerDefValidate(const virDomainControllerDef *controller)
>+{
>+    if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
>+        if (controller->idx > 255) {
>+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                           _("PCI controller index %d too high, maximum is 255"),
>+                           controller->idx);
>+            return -1;
>+        }
>+    }
>+    return 0;
>+}
>+
>+
>+static int
> virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
>                                    const virDomainDef *def)
> {
>@@ -4859,6 +4874,9 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
>     case VIR_DOMAIN_DEVICE_NET:
>         return virDomainNetDefValidate(dev->data.net);
>
>+    case VIR_DOMAIN_DEVICE_CONTROLLER:
>+        return virDomainControllerDefValidate(dev->data.controller);
>+
>     case VIR_DOMAIN_DEVICE_LEASE:
>     case VIR_DOMAIN_DEVICE_FS:
>     case VIR_DOMAIN_DEVICE_INPUT:
>@@ -4866,7 +4884,6 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
>     case VIR_DOMAIN_DEVICE_VIDEO:
>     case VIR_DOMAIN_DEVICE_HOSTDEV:
>     case VIR_DOMAIN_DEVICE_WATCHDOG:
>-    case VIR_DOMAIN_DEVICE_CONTROLLER:
>     case VIR_DOMAIN_DEVICE_GRAPHICS:
>     case VIR_DOMAIN_DEVICE_HUB:
>     case VIR_DOMAIN_DEVICE_SMARTCARD:
>--
>2.9.3
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list