Merge pull request #315 from avagin/nsenter

A few fixes for nsenter

namespaces/nsenter/nsenter.c

@@ -15,6 +15,8 @@
 #include <unistd.h>
 #include <getopt.h>
 
+#define pr_perror(fmt, ...) fprintf(stderr, "nsenter: " fmt ": %m\n", ##__VA_ARGS__)
+
 static const kBufSize = 256;
 static const char *kNsEnter = "nsenter";
 
@@ -22,6 +24,10 @@ void get_args(int *argc, char ***argv)
 {
 	// Read argv
 	int fd = open("/proc/self/cmdline", O_RDONLY);
+	if (fd < 0) {
+		pr_perror("Unable to open /proc/self/cmdline");
+		exit(1);
+	}
 
 	// Read the whole commandline.
 	ssize_t contents_size = 0;
@@ -34,6 +40,10 @@ void get_args(int *argc, char ***argv)
 		bytes_read =
 		    read(fd, contents + contents_offset,
 			 contents_size - contents_offset);
+		if (bytes_read < 0) {
+			pr_perror("Unable to read from /proc/self/cmdline");
+			exit(1);
+		}
 		contents_offset += bytes_read;
 	}
 	while (bytes_read > 0);
@@ -91,8 +101,7 @@ void nsenter()
 
 	#ifdef PR_SET_CHILD_SUBREAPER
 	if (prctl(PR_SET_CHILD_SUBREAPER, 1, 0, 0, 0) == -1) {
-		fprintf(stderr, "nsenter: failed to set child subreaper: %s",
+		pr_perror("Failed to set child subreaper");
-			strerror(errno));
 		exit(1);
 	}
 	#endif
@@ -124,9 +133,8 @@ void nsenter()
 
 	init_pid = strtol(init_pid_str, NULL, 10);
 	if ((init_pid == 0 && errno == EINVAL) || errno == ERANGE) {
-		fprintf(stderr,
+		pr_perror("Failed to parse PID from \"%s\" with output \"%d\"",
-			"nsenter: Failed to parse PID from \"%s\" with output \"%d\" and error: \"%s\"\n",
+			init_pid_str, init_pid);
-			init_pid_str, init_pid, strerror(errno));
 		print_usage();
 		exit(1);
 	}
@@ -135,7 +143,7 @@ void nsenter()
 	argv += 3;
 
 	if (setsid() == -1) {
-		fprintf(stderr, "setsid failed. Error: %s\n", strerror(errno));
+		pr_perror("setsid failed");
 		exit(1);
 	}
 	// before we setns we need to dup the console
@@ -143,9 +151,7 @@ void nsenter()
 	if (console != NULL) {
 		consolefd = open(console, O_RDWR);
 		if (consolefd < 0) {
-			fprintf(stderr,
+			pr_perror("Failed to open console %s", console);
-				"nsenter: failed to open console %s %s\n",
-				console, strerror(errno));
 			exit(1);
 		}
 	}
@@ -154,51 +160,60 @@ void nsenter()
 	memset(ns_dir, 0, PATH_MAX);
 	snprintf(ns_dir, PATH_MAX - 1, "/proc/%d/ns/", init_pid);
 
+	int ns_dir_fd;
+	ns_dir_fd = open(ns_dir, O_RDONLY | O_DIRECTORY);
+	if (ns_dir_fd < 0) {
+		pr_perror("Unable to open %s", ns_dir);
+		exit(1);
+	}
+
 	char *namespaces[] = { "ipc", "uts", "net", "pid", "mnt" };
 	const int num = sizeof(namespaces) / sizeof(char *);
 	int i;
 	for (i = 0; i < num; i++) {
-		char buf[PATH_MAX];
+		// A zombie process has links on namespaces, but they can't be opened
-		memset(buf, 0, PATH_MAX);
+		struct stat st;
-		snprintf(buf, PATH_MAX - 1, "%s%s", ns_dir, namespaces[i]);
+		if (fstatat(ns_dir_fd, namespaces[i], &st, AT_SYMLINK_NOFOLLOW) == -1) {
-		int fd = open(buf, O_RDONLY);
-		if (fd == -1) {
-			// Ignore nonexistent namespaces.
 			if (errno == ENOENT)
 				continue;
+			pr_perror("Failed to stat ns file %s for ns %s",
+				ns_dir, namespaces[i]);
+			exit(1);
+		}
 
-			fprintf(stderr,
+		int fd = openat(ns_dir_fd, namespaces[i], O_RDONLY);
-				"nsenter: Failed to open ns file \"%s\" for ns \"%s\" with error: \"%s\"\n",
+		if (fd == -1) {
-				buf, namespaces[i], strerror(errno));
+			pr_perror("Failed to open ns file %s for ns %s",
+				ns_dir, namespaces[i]);
 			exit(1);
 		}
 		// Set the namespace.
 		if (setns(fd, 0) == -1) {
-			fprintf(stderr,
+			pr_perror("Failed to setns for %s", namespaces[i]);
-				"nsenter: Failed to setns for \"%s\" with error: \"%s\"\n",
-				namespaces[i], strerror(errno));
 			exit(1);
 		}
 		close(fd);
 	}
+	close(ns_dir_fd);
 
 	// We must fork to actually enter the PID namespace.
 	int child = fork();
+	if (child == -1) {
+		pr_perror("Unable to fork a process");
+		exit(1);
+	}
 	if (child == 0) {
 		if (consolefd != -1) {
 			if (dup2(consolefd, STDIN_FILENO) != 0) {
-				fprintf(stderr, "nsenter: failed to dup 0 %s\n",
+				pr_perror("Failed to dup 0");
-					strerror(errno));
 				exit(1);
 			}
 			if (dup2(consolefd, STDOUT_FILENO) != STDOUT_FILENO) {
-				fprintf(stderr, "nsenter: failed to dup 1 %s\n",
+				pr_perror("Failed to dup 1");
-					strerror(errno));
 				exit(1);
 			}
 			if (dup2(consolefd, STDERR_FILENO) != STDERR_FILENO) {
-				fprintf(stderr, "nsenter: failed to dup 2 %s\n",
+				pr_perror("Failed to dup 2\n");
-					strerror(errno));
 				exit(1);
 			}
 		}
@@ -208,9 +223,7 @@ void nsenter()
 		// Parent, wait for the child.
 		int status = 0;
 		if (waitpid(child, &status, 0) == -1) {
-			fprintf(stderr,
+			pr_perror("nsenter: Failed to waitpid with error");
-				"nsenter: Failed to waitpid with error: \"%s\"\n",
-				strerror(errno));
 			exit(1);
 		}
 		// Forward the child's exit code or re-send its death signal.

namespaces/nsenter/nsenter_test.go

@@ -0,0 +1,70 @@
+package nsenter
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"os/signal"
+	"strings"
+	"syscall"
+	"testing"
+)
+
+func TestNsenterAlivePid(t *testing.T) {
+	args := []string{"nsenter-exec", "--nspid", fmt.Sprintf("%d", os.Getpid())}
+
+	cmd := &exec.Cmd{
+		Path: os.Args[0],
+		Args: args,
+	}
+
+	err := cmd.Run()
+	if err != nil {
+		t.Fatal("nsenter exits with a non-zero exit status")
+	}
+}
+
+func TestNsenterInvalidPid(t *testing.T) {
+	args := []string{"nsenter-exec", "--nspid", "-1"}
+
+	cmd := &exec.Cmd{
+		Path: os.Args[0],
+		Args: args,
+	}
+
+	err := cmd.Run()
+	if err == nil {
+		t.Fatal("nsenter exits with a zero exit status")
+	}
+}
+
+func TestNsenterDeadPid(t *testing.T) {
+
+	c := make(chan os.Signal)
+	signal.Notify(c, syscall.SIGCHLD)
+	dead_cmd := exec.Command("true")
+	if err := dead_cmd.Start(); err != nil {
+		t.Fatal(err)
+	}
+	defer dead_cmd.Wait()
+	<-c // dead_cmd is zombie
+
+	args := []string{"nsenter-exec", "--nspid", fmt.Sprintf("%d", dead_cmd.Process.Pid)}
+
+	cmd := &exec.Cmd{
+		Path: os.Args[0],
+		Args: args,
+	}
+
+	err := cmd.Run()
+	if err == nil {
+		t.Fatal("nsenter exits with a zero exit status")
+	}
+}
+
+func init() {
+	if strings.HasPrefix(os.Args[0], "nsenter-") {
+		os.Exit(0)
+	}
+	return
+}