[lxc-devel] [PATCH] Fix wrong calculations in clone_update_unexp_hooks()

[Christian Brauner]

(1) This commit fixes some long standing miscalculation when updating paths in
    lxc.hooks.* entries. We now also update conf->unexpandend_alloced which
    hasn't been done prior to this commit. According to my testing valgrind now
    no longer reports "x bytes definitely lost" when using lxc-clone.

(2) Also we use the stricter check:

    	if (p >= lend)
    		continue;

    This should deal better with invalid config files.

(3) Insert some spaces between operators to increase readability.

Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>
---
 src/lxc/confile.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index 47b0b8b..8f8a630 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -2701,7 +2701,8 @@ bool clone_update_unexp_ovl_paths(struct lxc_conf *conf, const char *oldpath,
 }
 
 bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,
-	const char *newpath, const char *oldname, const char *newname)
+			      const char *newpath, const char *oldname,
+			      const char *newname)
 {
 	const char *key = "lxc.hook";
 	int ret;
@@ -2711,13 +2712,13 @@ bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,
 	char *olddir = alloca(olddirlen + 1);
 	char *newdir = alloca(newdirlen + 1);
 
-	ret = snprintf(olddir, olddirlen+1, "%s/%s", oldpath, oldname);
-	if (ret < 0 || ret >= olddirlen+1) {
+	ret = snprintf(olddir, olddirlen + 1, "%s/%s", oldpath, oldname);
+	if (ret < 0 || ret >= olddirlen + 1) {
 		ERROR("Bug in %s", __func__);
 		return false;
 	}
-	ret = snprintf(newdir, newdirlen+1, "%s/%s", newpath, newname);
-	if (ret < 0 || ret >= newdirlen+1) {
+	ret = snprintf(newdir, newdirlen + 1, "%s/%s", newpath, newname);
+	if (ret < 0 || ret >= newdirlen + 1) {
 		ERROR("Bug in %s", __func__);
 		return false;
 	}
@@ -2733,7 +2734,7 @@ bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,
 			lstart = lend;
 			continue;
 		}
-		p = strchr(lstart+strlen(key), '=');
+		p = strchr(lstart + strlen(key), '=');
 		if (!p) {
 			lstart = lend;
 			continue;
@@ -2741,8 +2742,8 @@ bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,
 		p++;
 		while (isblank(*p))
 			p++;
-		if (!*p)
-			return true;
+                if (p >= lend)
+                        continue;
 		if (strncmp(p, olddir, strlen(olddir)) != 0) {
 			lstart = lend;
 			continue;
@@ -2753,6 +2754,8 @@ bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,
 			memcpy(p, newdir, newdirlen);
 			if (olddirlen != newdirlen) {
 				memmove(lend-diff, lend, strlen(lend)+1);
+				memmove(p + newdirlen, p + newdirlen + diff,
+					strlen(p) - newdirlen - diff + 1);
 				lend -= diff;
 				conf->unexpanded_len -= diff;
 			}
@@ -2763,7 +2766,7 @@ bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,
 			size_t oldlen = conf->unexpanded_len;
 			size_t newlen = oldlen + diff;
 			size_t poffset = p - conf->unexpanded_config;
-			new = realloc(conf->unexpanded_config, newlen);
+			new = realloc(conf->unexpanded_config, newlen + 1);
 			if (!new) {
 				ERROR("Out of memory");
 				return false;
@@ -2772,11 +2775,12 @@ bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,
 			new[newlen-1] = '\0';
 			lend = new + (lend - conf->unexpanded_config);
 			/* move over the remainder, /$hookname\n$rest */
-			memmove(new+poffset+newdirlen,
-					new+poffset+olddirlen,
-					oldlen-poffset-olddirlen);
+			memmove(new + poffset + newdirlen,
+				new + poffset + olddirlen,
+				oldlen - poffset - olddirlen + 1);
 			conf->unexpanded_config = new;
-			memcpy(new+poffset, newdir, newdirlen);
+			conf->unexpanded_alloced = newlen + 1;
+			memcpy(new + poffset, newdir, newdirlen);
 			lstart = lend + diff;
 		}
 	}
-- 
2.6.2