[PATCH v2] conf: fix integer overflow in virDomainControllerDefParseXML

Egor Makrushin posted 1 patch 4 months, 1 week ago
Failed in applying to current master (apply log)
src/conf/domain_conf.c  | 4 ++--
src/conf/domain_conf.h  | 2 +-
src/qemu/qemu_command.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
[PATCH v2] conf: fix integer overflow in virDomainControllerDefParseXML
Posted by Egor Makrushin 4 months, 1 week ago
Multiplication results in integer overflow.
Thus, replace it with ULLONG_MAX and change
def->opts.pciopts.pcihole64size type to ULL.
Update variable usage according to new type.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Egor Makrushin <emakrushin@astralinux.ru>
---
v2: update variable type according to maintainer's proposal
 src/conf/domain_conf.c  | 4 ++--
 src/conf/domain_conf.h  | 2 +-
 src/qemu/qemu_command.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 58a985fc5d..9f842937e6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8523,7 +8523,7 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt,
             unsigned long long bytes;
             if ((rc = virParseScaledValue("./pcihole64", NULL,
                                           ctxt, &bytes, 1024,
-                                          1024ULL * ULONG_MAX, false)) < 0)
+                                          ULLONG_MAX, false)) < 0)
                 return NULL;
 
             if (rc == 1)
@@ -23123,7 +23123,7 @@ virDomainControllerDefFormat(virBuffer *buf,
 
     if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
         def->opts.pciopts.pcihole64) {
-        virBufferAsprintf(&childBuf, "<pcihole64 unit='KiB'>%lu</"
+        virBufferAsprintf(&childBuf, "<pcihole64 unit='KiB'>%llu</"
                           "pcihole64>\n", def->opts.pciopts.pcihole64size);
     }
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0c5e2636e1..14901b37ba 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -707,7 +707,7 @@ struct _virDomainVirtioSerialOpts {
 
 struct _virDomainPCIControllerOpts {
     bool pcihole64;
-    unsigned long pcihole64size;
+    unsigned long long pcihole64size;
 
     /* the exact controller name is in the "model" subelement, e.g.:
      * <controller type='pci' model='pcie-root-port'>
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 54fb8220e8..b45a5e4f80 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6211,7 +6211,7 @@ qemuBuildGlobalControllerCommandLine(virCommand *cmd,
             }
 
             virCommandAddArg(cmd, "-global");
-            virCommandAddArgFormat(cmd, "%s.pci-hole64-size=%luK", hoststr,
+            virCommandAddArgFormat(cmd, "%s.pci-hole64-size=%lluK", hoststr,
                                    cont->opts.pciopts.pcihole64size);
         }
     }
-- 
2.30.2
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v2] conf: fix integer overflow in virDomainControllerDefParseXML
Posted by Michal Prívozník 4 months, 1 week ago
On 12/20/23 13:38, Egor Makrushin wrote:
> Multiplication results in integer overflow.
> Thus, replace it with ULLONG_MAX and change
> def->opts.pciopts.pcihole64size type to ULL.
> Update variable usage according to new type.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Egor Makrushin <emakrushin@astralinux.ru>
> ---
> v2: update variable type according to maintainer's proposal
>  src/conf/domain_conf.c  | 4 ++--
>  src/conf/domain_conf.h  | 2 +-
>  src/qemu/qemu_command.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 58a985fc5d..9f842937e6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8523,7 +8523,7 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt,
>              unsigned long long bytes;
>              if ((rc = virParseScaledValue("./pcihole64", NULL,
>                                            ctxt, &bytes, 1024,
> -                                          1024ULL * ULONG_MAX, false)) < 0)
> +                                          ULLONG_MAX, false)) < 0)
>                  return NULL;
>  
>              if (rc == 1)
> @@ -23123,7 +23123,7 @@ virDomainControllerDefFormat(virBuffer *buf,
>  
>      if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
>          def->opts.pciopts.pcihole64) {
> -        virBufferAsprintf(&childBuf, "<pcihole64 unit='KiB'>%lu</"
> +        virBufferAsprintf(&childBuf, "<pcihole64 unit='KiB'>%llu</"
>                            "pcihole64>\n", def->opts.pciopts.pcihole64size);

Let's take this opportunity and fix this crazy fmt string.

>      }
>  
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 0c5e2636e1..14901b37ba 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -707,7 +707,7 @@ struct _virDomainVirtioSerialOpts {
>  
>  struct _virDomainPCIControllerOpts {
>      bool pcihole64;
> -    unsigned long pcihole64size;
> +    unsigned long long pcihole64size;
>  
>      /* the exact controller name is in the "model" subelement, e.g.:
>       * <controller type='pci' model='pcie-root-port'>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 54fb8220e8..b45a5e4f80 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6211,7 +6211,7 @@ qemuBuildGlobalControllerCommandLine(virCommand *cmd,
>              }
>  
>              virCommandAddArg(cmd, "-global");
> -            virCommandAddArgFormat(cmd, "%s.pci-hole64-size=%luK", hoststr,
> +            virCommandAddArgFormat(cmd, "%s.pci-hole64-size=%lluK", hoststr,
>                                     cont->opts.pciopts.pcihole64size);
>          }
>      }

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed. Congratulations on your first libvirt contribution!

Michal
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org