[libvirt] [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, 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

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

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

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
participants (2)
-
Laine Stump
-
Martin Kletzander