From 0657ed2a5c2499e41b4797ccd5522ad7cab13a20 Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Mon, 16 Jun 2014 17:18:07 +0200
Subject: [PATCH] net: leaseshelper: Refactor copying of old entries to avoid
 double free

When copying entries from the old lease file into the new array the old
code would copy the pointer of the json object into the second array
without removing it from the first. Afterwards when both arrays were
freed this might lead to a crash due to access of already freed memory.

Refactor the code to use the new array item stealing helper added to the
json code so that the entry resides just in one array.
---
 src/network/leaseshelper.c | 79 +++++++++++++++++++++-----------------
 1 file changed, 43 insertions(+), 36 deletions(-)

diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c
index d4b48bbf51..e4b5283f8b 100644
--- a/src/network/leaseshelper.c
+++ b/src/network/leaseshelper.c
@@ -116,7 +116,6 @@ main(int argc, char **argv)
     long long currtime = 0;
     long long expirytime = 0;
     size_t i = 0;
-    int size = 0;
     int action = -1;
     int pid_file_fd = -1;
     int rv = EXIT_FAILURE;
@@ -270,6 +269,15 @@ main(int argc, char **argv)
                 virLeaseActionTypeToString(action));
         exit(EXIT_FAILURE);
     }
+
+    if (!(leases_array_new = virJSONValueNewArray())) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("failed to create json"));
+        goto cleanup;
+    }
+
+    currtime = (long long) time(NULL);
+
     /* Check for previous leases */
     if (custom_lease_file_len) {
         if (!(leases_array = virJSONValueFromString(lease_entries))) {
@@ -277,52 +285,51 @@ main(int argc, char **argv)
                            _("invalid json in file: %s, rewriting it"),
                            custom_lease_file);
         } else {
-            if ((size = virJSONValueArraySize(leases_array)) < 0) {
+            if (!virJSONValueIsArray(leases_array)) {
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("couldn't fetch array of leases"));
                 goto cleanup;
             }
-        }
-    }
-
-    if (!(leases_array_new = virJSONValueNewArray())) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("failed to create json"));
-        goto cleanup;
-    }
 
-    currtime = (long long) time(NULL);
+            i = 0;
+            while (i < virJSONValueArraySize(leases_array)) {
+                const char *ip_tmp = NULL;
+                long long expirytime_tmp = -1;
 
-    for (i = 0; i < size; i++) {
-        const char *ip_tmp = NULL;
-        long long expirytime_tmp = -1;
+                if (!(lease_tmp = virJSONValueArrayGet(leases_array, i))) {
+                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                   _("failed to parse json"));
+                    goto cleanup;
+                }
 
-        if (!(lease_tmp = virJSONValueArrayGet(leases_array, i))) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("failed to parse json"));
-            goto cleanup;
-        }
+                if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address")) ||
+                    (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time", &expirytime_tmp) < 0)) {
+                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                   _("failed to parse json"));
+                    goto cleanup;
+                }
 
-        if (!(ip_tmp = virJSONValueObjectGetString(lease_tmp, "ip-address")) ||
-            (virJSONValueObjectGetNumberLong(lease_tmp, "expiry-time", &expirytime_tmp) < 0)) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("failed to parse json"));
-            goto cleanup;
-        }
+                /* Check whether lease has expired or not */
+                if (expirytime_tmp < currtime) {
+                    i++;
+                    continue;
+                }
 
-        /* Check whether lease has expired or not */
-        if (expirytime_tmp < currtime)
-            continue;
+                /* Check whether lease has to be included or not */
+                if (delete && STREQ(ip_tmp, ip)) {
+                    i++;
+                    continue;
+                }
 
-        /* Check whether lease has to be included or not */
-        if (delete && STREQ(ip_tmp, ip))
-            continue;
+                /* Move old lease to new array */
+                if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) {
+                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                   _("failed to create json"));
+                    goto cleanup;
+                }
 
-        /* Add old lease to new array */
-        if (virJSONValueArrayAppend(leases_array_new, lease_tmp) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("failed to create json"));
-            goto cleanup;
+                ignore_value(virJSONValueArraySteal(leases_array, i));
+            }
         }
     }
 
-- 
GitLab