[PATCH v2] virsh: fix broken code in freepages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Commit 9e3efe53 broke the build under valgrind or clang, by writing
8 bytes through an allocation of 4 bytes.  It also risks multiplication
overflow when mallocing (that's a pervasive problem that needs an
audit in the rest of the code, but we might as well fix this one while
we are here), and had a typo.

* tools/virsh-host.c (cmdFreepages): Avoid integer overflow and
undefined behavior.

Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---

Pushing under the build-breaker rule.  v1 was:
https://www.redhat.com/archives/libvir-list/2014-June/msg00937.html

 tools/virsh-host.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 2d6cb00..13d4c5c 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -1,7 +1,7 @@
 /*
  * virsh-host.c: Commands in "Host and Hypervisor" group.
  *
- * Copyright (C) 2005, 2007-2012 Red Hat, Inc.
+ * Copyright (C) 2005, 2007-2014 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -214,7 +214,7 @@ static const vshCmdOptDef opts_freepages[] = {
     },
     {.name = "pagesize",
      .type = VSH_OT_INT,
-     .help = N_("page size (in kibibites)")
+     .help = N_("page size (in kibibytes)")
     },
     {.name = "all",
      .type = VSH_OT_BOOL,
@@ -229,6 +229,7 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
     bool ret = false;
     unsigned int npages;
     unsigned int *pagesize = NULL;
+    unsigned long long tmp;
     int cell;
     unsigned long long *counts = NULL;
     size_t i, j;
@@ -261,7 +262,7 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
             goto cleanup;
         }

-        pagesize = vshMalloc(ctl, nodes_cnt * sizeof(*pagesize));
+        pagesize = vshCalloc(ctl, nodes_cnt, sizeof(*pagesize));

         for (i = 0; i < nodes_cnt; i++) {
             char *val = virXMLPropString(nodes[i], "size");
@@ -278,7 +279,7 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
         npages = nodes_cnt;
         VIR_FREE(nodes);

-        counts = vshMalloc(ctl, npages * sizeof(*counts));
+        counts = vshCalloc(ctl, npages, sizeof(*counts));

         nodes_cnt = virXPathNodeSet("/capabilities/host/topology/cells/cell",
                                     ctxt, &nodes);
@@ -319,15 +320,13 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
             goto cleanup;
         }

-        pagesize = vshMalloc(ctl, sizeof(*pagesize));
-        if (vshCommandOptScaledInt(cmd, "pagesize", (unsigned long long *) pagesize,
-                                   1, UINT_MAX) < 0) {
+        if (vshCommandOptScaledInt(cmd, "pagesize", &tmp, 1, UINT_MAX) < 0) {
             vshError(ctl, "%s", _("page size has to be a number"));
             goto cleanup;
         }
-
         /* page size is expected in kibibytes */
-        pagesize[0] /= 1024;
+        pagesize = vshMalloc(ctl, sizeof(*pagesize));
+        *pagesize = tmp / 1024;

         if (!pagesize[0]) {
             vshError(ctl, "%s", _("page size must be at least 1KiB"));
-- 
1.9.3

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]