[PATCH 2/4] src: validate permitted ACPI table types in libxl/qemu drivers

Daniel P. Berrangé posted 4 patches 10 months ago
[PATCH 2/4] src: validate permitted ACPI table types in libxl/qemu drivers
Posted by Daniel P. Berrangé 10 months ago
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/libxl/libxl_domain.c | 14 ++++++++++++++
 src/qemu/qemu_validate.c | 15 +++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 6805160923..816ed2f349 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -306,6 +306,7 @@ libxlDomainDefValidate(const virDomainDef *def,
     libxlDriverPrivate *driver = opaque;
     g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver);
     bool reqSecureBoot = false;
+    size_t i;
 
     if (!virCapabilitiesDomainSupported(cfg->caps, def->os.type,
                                         def->os.arch,
@@ -330,6 +331,19 @@ libxlDomainDefValidate(const virDomainDef *def,
         return -1;
     }
 
+    for (i = 0; i < def->os.nacpiTables; i++) {
+        switch ((virDomainOsACPITable)def->os.acpiTables[i]->type) {
+        case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC:
+            break;
+
+        default:
+        case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_LAST:
+            virReportEnumRangeError(virDomainOsACPITable,
+                                    def->os.acpiTables[i]->type);
+            return -1;
+        }
+    }
+
     if (def->nsounds > 0) {
         virDomainSoundDef *snd = def->sounds[0];
 
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 3e3e368da3..039f5f84e6 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -701,6 +701,8 @@ static int
 qemuValidateDomainDefBoot(const virDomainDef *def,
                           virQEMUCaps *qemuCaps)
 {
+    size_t i;
+
     if (def->os.bootloader || def->os.bootloaderArgs) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("bootloader is not supported by QEMU"));
@@ -740,6 +742,19 @@ qemuValidateDomainDefBoot(const virDomainDef *def,
             return -1;
     }
 
+    for (i = 0; i < def->os.nacpiTables; i++) {
+        switch ((virDomainOsACPITable)def->os.acpiTables[i]->type) {
+        case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC:
+            break;
+
+        default:
+        case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_LAST:
+            virReportEnumRangeError(virDomainOsACPITable,
+                                    def->os.acpiTables[i]->type);
+            return -1;
+        }
+    }
+
     return 0;
 }
 
-- 
2.47.1
Re: [PATCH 2/4] src: validate permitted ACPI table types in libxl/qemu drivers
Posted by Michal Prívozník 10 months ago
On 2/18/25 19:12, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/libxl/libxl_domain.c | 14 ++++++++++++++
>  src/qemu/qemu_validate.c | 15 +++++++++++++++
>  2 files changed, 29 insertions(+)
> 

Please consider squashing in the following:


diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 816ed2f349..0eb414d20d 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -332,7 +332,7 @@ libxlDomainDefValidate(const virDomainDef *def,
     }
 
     for (i = 0; i < def->os.nacpiTables; i++) {
-        switch ((virDomainOsACPITable)def->os.acpiTables[i]->type) {
+        switch (def->os.acpiTables[i]->type) {
         case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC:
             break;
 
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 039f5f84e6..3744252284 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -743,7 +743,7 @@ qemuValidateDomainDefBoot(const virDomainDef *def,
     }
 
     for (i = 0; i < def->os.nacpiTables; i++) {
-        switch ((virDomainOsACPITable)def->os.acpiTables[i]->type) {
+        switch (def->os.acpiTables[i]->type) {
         case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC:
             break;


Michal
Re: [PATCH 2/4] src: validate permitted ACPI table types in libxl/qemu drivers
Posted by Daniel P. Berrangé 10 months ago
On Wed, Feb 19, 2025 at 04:57:07PM +0100, Michal Prívozník wrote:
> On 2/18/25 19:12, Daniel P. Berrangé wrote:
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  src/libxl/libxl_domain.c | 14 ++++++++++++++
> >  src/qemu/qemu_validate.c | 15 +++++++++++++++
> >  2 files changed, 29 insertions(+)
> > 
> 
> Please consider squashing in the following:
> 
> 
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 816ed2f349..0eb414d20d 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -332,7 +332,7 @@ libxlDomainDefValidate(const virDomainDef *def,
>      }
>  
>      for (i = 0; i < def->os.nacpiTables; i++) {
> -        switch ((virDomainOsACPITable)def->os.acpiTables[i]->type) {
> +        switch (def->os.acpiTables[i]->type) {
>          case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC:
>              break;
>  
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 039f5f84e6..3744252284 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -743,7 +743,7 @@ qemuValidateDomainDefBoot(const virDomainDef *def,
>      }
>  
>      for (i = 0; i < def->os.nacpiTables; i++) {
> -        switch ((virDomainOsACPITable)def->os.acpiTables[i]->type) {
> +        switch (def->os.acpiTables[i]->type) {
>          case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC:
>              break;

Why do that ?  This means we won't get warned to double check validation
when adding new constants.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH 2/4] src: validate permitted ACPI table types in libxl/qemu drivers
Posted by Michal Prívozník 10 months ago
On 2/19/25 17:01, Daniel P. Berrangé wrote:
> On Wed, Feb 19, 2025 at 04:57:07PM +0100, Michal Prívozník wrote:
>> On 2/18/25 19:12, Daniel P. Berrangé wrote:
>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>> ---
>>>  src/libxl/libxl_domain.c | 14 ++++++++++++++
>>>  src/qemu/qemu_validate.c | 15 +++++++++++++++
>>>  2 files changed, 29 insertions(+)
>>>
>>
>> Please consider squashing in the following:
>>
>>
>> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
>> index 816ed2f349..0eb414d20d 100644
>> --- a/src/libxl/libxl_domain.c
>> +++ b/src/libxl/libxl_domain.c
>> @@ -332,7 +332,7 @@ libxlDomainDefValidate(const virDomainDef *def,
>>      }
>>  
>>      for (i = 0; i < def->os.nacpiTables; i++) {
>> -        switch ((virDomainOsACPITable)def->os.acpiTables[i]->type) {
>> +        switch (def->os.acpiTables[i]->type) {
>>          case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC:
>>              break;
>>  
>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>> index 039f5f84e6..3744252284 100644
>> --- a/src/qemu/qemu_validate.c
>> +++ b/src/qemu/qemu_validate.c
>> @@ -743,7 +743,7 @@ qemuValidateDomainDefBoot(const virDomainDef *def,
>>      }
>>  
>>      for (i = 0; i < def->os.nacpiTables; i++) {
>> -        switch ((virDomainOsACPITable)def->os.acpiTables[i]->type) {
>> +        switch (def->os.acpiTables[i]->type) {
>>          case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC:
>>              break;
> 
> Why do that ?  This means we won't get warned to double check validation
> when adding new constants.

We will since in the previous patch I've suggested to turn ->type into
its proper enum type instead of int.

Michal