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

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

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@xxxxxxxxxx
> > > https://www.redhat.com/mailman/listinfo/libvir-list

> From 4416cde9d9ce36581b56a3de905122c54c2cfb0e Mon Sep 17 00:00:00 2001
> From: Laine Stump <laine@xxxxxxxxx>
> 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

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list