conf/ile: make sure buffer is large enough

conf.c: In function 'lxc_assign_network':
conf.c:3096:25: error: '%lu' directive output may be truncated writing between 1 and 20 bytes into a region of size 19 [-Werror=format-truncation=]
   snprintf(pidstr, 19, "%lu", (unsigned long) pid);
                         ^~~
conf.c:3096:24: note: using the range [1, 18446744073709551615] for directive argument
   snprintf(pidstr, 19, "%lu", (unsigned long) pid);
                        ^~~~~
In file included from /usr/include/stdio.h:938:0,
                 from conf.c:35:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:64:10: note: format output between 2 and 21 bytes into a destination of size 19
   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        __bos (__s), __fmt, __va_arg_pack ());
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
confile.c: In function 'network_new_hwaddrs':
confile.c:2889:38: error: '%02x' directive output may be truncated writing between 2 and 8 bytes into a region of size 6 [-Werror=format-truncation=]
  snprintf(hwaddr, 18, "00:16:3e:%02x:%02x:%02x",
                                      ^~~~
confile.c:2889:23: note: using the range [0, 4294967295] for directive argument
  snprintf(hwaddr, 18, "00:16:3e:%02x:%02x:%02x",
                       ^~~~~~~~~~~~~~~~~~~~~~~~~
confile.c:2889:23: note: using the range [0, 4294967295] for directive argument
In file included from /usr/include/stdio.h:938:0,
                 from confile.c:24:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:64:10: note: format output between 18 and 36 bytes into a destination of size 18
   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        __bos (__s), __fmt, __va_arg_pack ());
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Not sure whether the latter is really a problem. We might need an additional
fix later on.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

src/lxc/conf.c

@@ -3058,56 +3058,67 @@ static int unpriv_assign_nic(const char *lxcpath, char *lxcname,
 	int bytes, pipefd[2];
 	char *token, *saveptr = NULL;
 	char buffer[MAX_BUFFER_SIZE];
-	char netdev_link[IFNAMSIZ+1];
+	char netdev_link[IFNAMSIZ + 1];
 
 	if (netdev->type != LXC_NET_VETH) {
 		ERROR("nic type %d not support for unprivileged use",
-			netdev->type);
+		      netdev->type);
 		return -1;
 	}
 
-	if(pipe(pipefd) < 0) {
+	if (pipe(pipefd) < 0) {
 		SYSERROR("pipe failed");
 		return -1;
 	}
 
-	if ((child = fork()) < 0) {
+	child = fork();
+	if (child < 0) {
 		SYSERROR("fork");
 		close(pipefd[0]);
 		close(pipefd[1]);
 		return -1;
 	}
 
 	if (child == 0) { // child
-		/* close the read-end of the pipe */
-		close(pipefd[0]);
-		/* redirect the stdout to write-end of the pipe */
-		dup2(pipefd[1], STDOUT_FILENO);
-		/* close the write-end of the pipe */
-		close(pipefd[1]);
+		/* Call lxc-user-nic pid type bridge. */
+		int ret;
+		char pidstr[LXC_NUMSTRLEN64];
+
+		close(pipefd[0]); /* Close the read-end of the pipe. */
+
+		/* Redirect stdout to write-end of the pipe. */
+		ret = dup2(pipefd[1], STDOUT_FILENO);
+		close(pipefd[1]); /* Close the write-end of the pipe. */
+		if (ret < 0) {
+			SYSERROR("Failed to dup2() to redirect stdout to pipe file descriptor.");
+			exit(EXIT_FAILURE);
+		}
 
-		// Call lxc-user-nic pid type bridge
-		char pidstr[20];
-		if (netdev->link) {
+		if (netdev->link)
 			strncpy(netdev_link, netdev->link, IFNAMSIZ);
-		} else {
+		else
 			strncpy(netdev_link, "none", IFNAMSIZ);
-		}
-		snprintf(pidstr, 19, "%lu", (unsigned long) pid);
-		pidstr[19] = '\0';
+
+		ret = snprintf(pidstr, LXC_NUMSTRLEN64, "%d", pid);
+		if (ret < 0 || ret >= LXC_NUMSTRLEN64)
+			exit(EXIT_FAILURE);
+		pidstr[LXC_NUMSTRLEN64 - 1] = '\0';
+
+		INFO("Execing lxc-user-nic %s %s %s veth %s %s", lxcpath,
+		     lxcname, pidstr, netdev_link, netdev->name);
 		execlp(LXC_USERNIC_PATH, LXC_USERNIC_PATH, lxcpath, lxcname,
-				pidstr, "veth", netdev_link, netdev->name, NULL);
-		SYSERROR("execvp lxc-user-nic");
-		exit(1);
+		       pidstr, "veth", netdev_link, netdev->name, NULL);
+
+		SYSERROR("Failed to exec lxc-user-nic.");
+		exit(EXIT_FAILURE);
 	}
 
 	/* close the write-end of the pipe */
 	close(pipefd[1]);
 
 	bytes = read(pipefd[0], &buffer, MAX_BUFFER_SIZE);
-	if (bytes < 0) {
-		SYSERROR("read failed");
-	}
+	if (bytes < 0)
+		SYSERROR("Failed to read from pipe file descriptor.");
 	buffer[bytes - 1] = '\0';
 
 	if (wait_for_pid(child) != 0) {
@@ -3122,21 +3133,23 @@ static int unpriv_assign_nic(const char *lxcpath, char *lxcname,
 	token = strtok_r(buffer, ":", &saveptr);
 	if (!token)
 		return -1;
-	netdev->name = malloc(IFNAMSIZ+1);
+
+	netdev->name = malloc(IFNAMSIZ + 1);
 	if (!netdev->name) {
-		ERROR("Out of memory");
+		SYSERROR("Failed to allocate memory.");
 		return -1;
 	}
-	memset(netdev->name, 0, IFNAMSIZ+1);
+	memset(netdev->name, 0, IFNAMSIZ + 1);
 	strncpy(netdev->name, token, IFNAMSIZ);
 
 	/* fill netdev->veth_attr.pair field */
 	token = strtok_r(NULL, ":", &saveptr);
 	if (!token)
 		return -1;
+
 	netdev->priv.veth_attr.pair = strdup(token);
 	if (!netdev->priv.veth_attr.pair) {
-		ERROR("Out of memory");
+		ERROR("Failed to allocate memory.");
 		return -1;
 	}
 

src/lxc/confile.c

@@ -725,7 +725,7 @@ static int create_matched_ifnames(const char *value, struct lxc_conf *lxc_conf)
 
 	freeifaddrs(ifaddr); /* free the dynamic memory */
 	ifaddr = NULL;	    /* prevent use after free */
-	
+
 	return ret;
 }
 
@@ -2957,21 +2957,21 @@ bool clone_update_unexp_hooks(struct lxc_conf *conf, const char *oldpath,
 	} \
 }
 
-static void new_hwaddr(char *hwaddr)
+static bool new_hwaddr(char *hwaddr)
 {
-	FILE *f;
-	f = fopen("/dev/urandom", "r");
-	if (f) {
-		unsigned int seed;
-		int ret = fread(&seed, sizeof(seed), 1, f);
-		if (ret != 1)
-			seed = time(NULL);
-		fclose(f);
-		srand(seed);
-	} else
-		srand(time(NULL));
-	snprintf(hwaddr, 18, "00:16:3e:%02x:%02x:%02x",
-			rand() % 255, rand() % 255, rand() % 255);
+	int ret;
+
+	/* COMMENT(brauner): Initialize random number generator. */
+	(void)randseed(true);
+
+	ret = snprintf(hwaddr, 18, "00:16:3e:%02x:%02x:%02x", rand() % 255,
+		       rand() % 255, rand() % 255);
+	if (ret < 0 || ret >= 18) {
+		SYSERROR("Failed to call snprintf().");
+		return false;
+	}
+
+	return true;
 }
 
 /*
@@ -2993,27 +2993,33 @@ bool network_new_hwaddrs(struct lxc_conf *conf)
 
 	if (!conf->unexpanded_config)
 		return true;
+
 	while (*lstart) {
 		char newhwaddr[18], oldhwaddr[17];
+
 		lend = strchr(lstart, '\n');
 		if (!lend)
 			lend = lstart + strlen(lstart);
 		else
 			lend++;
+
 		if (strncmp(lstart, key, strlen(key)) != 0) {
 			lstart = lend;
 			continue;
 		}
+
 		p = strchr(lstart+strlen(key), '=');
 		if (!p) {
 			lstart = lend;
 			continue;
 		}
+
 		p++;
 		while (isblank(*p))
 			p++;
 		if (!*p)
 			return true;
+
 		p2 = p;
 		while (*p2 && !isblank(*p2) && *p2 != '\n')
 			p2++;
@@ -3022,8 +3028,12 @@ bool network_new_hwaddrs(struct lxc_conf *conf)
 			lstart = lend;
 			continue;
 		}
+
 		memcpy(oldhwaddr, p, 17);
-		new_hwaddr(newhwaddr);
+
+		if (!new_hwaddr(newhwaddr))
+			return false;
+
 		memcpy(p, newhwaddr, 17);
 		lxc_list_for_each(it, &conf->network) {
 			struct lxc_netdev *n = it->elem;
@@ -3033,6 +3043,7 @@ bool network_new_hwaddrs(struct lxc_conf *conf)
 
 		lstart = lend;
 	}
+
 	return true;
 }
 

src/lxc/utils.c

@@ -1014,7 +1014,7 @@ int randseed(bool srand_it)
 	/*
 	   srand pre-seed function based on /dev/urandom
 	   */
-	unsigned int seed=time(NULL)+getpid();
+	unsigned int seed = time(NULL) + getpid();
 
 	FILE *f;
 	f = fopen("/dev/urandom", "r");