[lxc-devel] [lxd/master] main nsexec cgo allocation fixes

[brauner on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 370 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170211/c3f113a2/attachment.bin>
-------------- next part --------------
From 839505976b4a0d6058c48d591b830e2152d42c9e Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sat, 11 Feb 2017 02:12:20 +0100
Subject: [PATCH 1/2] cgo: close *DIR stream returned by fdopendir()

While it is true that the file descriptor used in the call to fdopendir() is not
supposed to be used anymore after a successful call, the *DIR stream returned by
fdopendir() must be closed.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/main_nsexec.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lxd/main_nsexec.go b/lxd/main_nsexec.go
index 49af9d2..34cbe5a 100644
--- a/lxd/main_nsexec.go
+++ b/lxd/main_nsexec.go
@@ -328,7 +328,8 @@ int manip_file_in_ns(char *rootfs, int pid, char *host, char *container, bool is
 				fprintf(stderr, "\n");
 			}
 
-			// container_fd is dead now that we fopendir'd it
+			closedir(fdir);
+			// container_fd is dead now that we fdopendir'd it
 			goto close_host;
 		} else {
 			fprintf(stderr, "type: file\n");

From 086acc50193e832ddbe6b1e983cce38640e285ef Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sat, 11 Feb 2017 02:21:51 +0100
Subject: [PATCH 2/2] cgo: free allocated memory

Well it is surprising that this bug didn't blow up in our face before. A couple
things:

- the pointer returned by dirname() is not allowed to be free()ed
  (See man 3 dirname)
- strdup() was not checked for NULL
- strdup() and dirname() assigned to the same variable
- Even though the pointer returned by dirname() was not supposed to be free()ed
  we were lucky enough to not be bitten by this since the free() call was
  basically a noop thanks to the switch statement.

Do none of these things!

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 lxd/main_nsexec.go | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lxd/main_nsexec.go b/lxd/main_nsexec.go
index 34cbe5a..a1239f4 100644
--- a/lxd/main_nsexec.go
+++ b/lxd/main_nsexec.go
@@ -394,21 +394,27 @@ void ensure_file(char *dest) {
 }
 
 void create(char *src, char *dest) {
+	char *dirdup;
 	char *destdirname;
+
 	struct stat sb;
 	if (stat(src, &sb) < 0) {
 		fprintf(stderr, "source %s does not exist\n", src);
 		_exit(1);
 	}
 
-	destdirname = strdup(dest);
-	destdirname = dirname(destdirname);
+	dirdup = strdup(dest);
+	if (!dirdup)
+		_exit(1);
+
+	destdirname = dirname(dirdup);
 
 	if (mkdir_p(destdirname, 0755) < 0) {
 		fprintf(stderr, "failed to create path: %s\n", destdirname);
-		free(destdirname);
+		free(dirdup);
 		_exit(1);
 	}
+	free(dirdup);
 
 	switch (sb.st_mode & S_IFMT) {
 	case S_IFDIR:
@@ -418,8 +424,6 @@ void create(char *src, char *dest) {
 		ensure_file(dest);
 		return;
 	}
-
-	free(destdirname);
 }
 
 void forkmount(char *buf, char *cur, ssize_t size) {