From f385dd3a47894bceb47f15ac07092985ada68f1e Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Thu, 15 Aug 2019 11:32:28 +0200
Subject: [PATCH] qemu: monitor: Change fields in qemuBlockStats to 'unsigned'
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

None of the fields actually return negative values. The internal
implementation of BlockAcctStats struct in qemu uses uint64_t and the
last place using -1 in libvirt was in the HMP monitor code which was
deleted.

Change the internal type to unsigned long long and ensure that all
public conversions don't overflow.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 src/qemu/qemu_driver.c       | 35 ++++++++++++++++++++++-------------
 src/qemu/qemu_monitor.h      | 16 ++++++++--------
 src/qemu/qemu_monitor_json.c |  2 +-
 3 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5dd31ebf16..ee0b817a0d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11417,8 +11417,7 @@ qemuDomainBlockStatsGatherTotals(qemuBlockStatsPtr data,
                                  qemuBlockStatsPtr total)
 {
 #define QEMU_BLOCK_STAT_TOTAL(NAME) \
-    if (data->NAME > 0) \
-        total->NAME += data->NAME
+    total->NAME += data->NAME
 
     QEMU_BLOCK_STAT_TOTAL(wr_bytes);
     QEMU_BLOCK_STAT_TOTAL(wr_req);
@@ -11574,10 +11573,14 @@ qemuDomainBlockStats(virDomainPtr dom,
     if (qemuDomainBlocksStatsGather(driver, vm, path, false, &blockstats) < 0)
         goto endjob;
 
-    stats->rd_req = blockstats->rd_req;
-    stats->rd_bytes = blockstats->rd_bytes;
-    stats->wr_req = blockstats->wr_req;
-    stats->wr_bytes = blockstats->wr_bytes;
+    if (VIR_ASSIGN_IS_OVERFLOW(stats->rd_req, blockstats->rd_req) ||
+        VIR_ASSIGN_IS_OVERFLOW(stats->rd_bytes, blockstats->rd_bytes) ||
+        VIR_ASSIGN_IS_OVERFLOW(stats->wr_req, blockstats->wr_req) ||
+        VIR_ASSIGN_IS_OVERFLOW(stats->wr_bytes, blockstats->wr_bytes)) {
+        virReportError(VIR_ERR_OVERFLOW, "%s", _("statistic value too large"));
+        goto endjob;
+    }
+
     /* qemu doesn't report the error count */
     stats->errs = -1;
 
@@ -11639,9 +11642,15 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
     nstats = 0;
 
 #define QEMU_BLOCK_STATS_ASSIGN_PARAM(VAR, NAME) \
-    if (nstats < *nparams && (blockstats->VAR) != -1) { \
+    if (nstats < *nparams) { \
+        long long tmp; \
+        if (VIR_ASSIGN_IS_OVERFLOW(tmp, (blockstats->VAR))) { \
+            virReportError(VIR_ERR_OVERFLOW, \
+                           _("value of '%s' is too large"), NAME); \
+            goto endjob; \
+        } \
         if (virTypedParameterAssign(params + nstats, NAME, \
-                                    VIR_TYPED_PARAM_LLONG, (blockstats->VAR)) < 0) \
+                                    VIR_TYPED_PARAM_LLONG, tmp) < 0) \
             goto endjob; \
         nstats++; \
     }
@@ -21479,11 +21488,11 @@ do { \
     char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \
     snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \
              "block.%zu.%s", num, name); \
-    if (value >= 0 && virTypedParamsAddULLong(&(record)->params, \
-                                              &(record)->nparams, \
-                                              maxparams, \
-                                              param_name, \
-                                              value) < 0) \
+    if (virTypedParamsAddULLong(&(record)->params, \
+                                &(record)->nparams, \
+                                maxparams, \
+                                param_name, \
+                                value) < 0) \
         goto cleanup; \
 } while (0)
 
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index b781a8cc89..8fc11c955e 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -642,14 +642,14 @@ virJSONValuePtr qemuMonitorQueryBlockstats(qemuMonitorPtr mon);
 typedef struct _qemuBlockStats qemuBlockStats;
 typedef qemuBlockStats *qemuBlockStatsPtr;
 struct _qemuBlockStats {
-    long long rd_req;
-    long long rd_bytes;
-    long long wr_req;
-    long long wr_bytes;
-    long long rd_total_times;
-    long long wr_total_times;
-    long long flush_req;
-    long long flush_total_times;
+    unsigned long long rd_req;
+    unsigned long long rd_bytes;
+    unsigned long long wr_req;
+    unsigned long long wr_bytes;
+    unsigned long long rd_total_times;
+    unsigned long long wr_total_times;
+    unsigned long long flush_req;
+    unsigned long long flush_total_times;
     unsigned long long capacity;
     unsigned long long physical;
 
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a590bd7d6a..cdfaf9785a 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2545,7 +2545,7 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValuePtr dev,
 #define QEMU_MONITOR_BLOCK_STAT_GET(NAME, VAR, MANDATORY) \
     if (MANDATORY || virJSONValueObjectHasKey(stats, NAME)) { \
         (*nstats)++; \
-        if (virJSONValueObjectGetNumberLong(stats, NAME, &VAR) < 0) { \
+        if (virJSONValueObjectGetNumberUlong(stats, NAME, &VAR) < 0) { \
             virReportError(VIR_ERR_INTERNAL_ERROR, \
                            _("cannot read %s statistic"), NAME); \
             return NULL; \
-- 
GitLab