[CRIU] [PATCH v2] remap: fix dead pid remap of /proc/<pid>

[Tycho Andersen]

It turns out we can't just test for /proc/<pid>, because the kernel appends
(deleted), since the directory is actually deleted (vs. something like
/proc/1/mountinfo, where the file still exists in the unlinked directory,
so there is no (deleted)). See comment for details.

v2: s/ret/is_dead in /proc/<pid>/xxx test, split tests to correctly test
    both cases

Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
---
 files-reg.c                                    | 34 ++++++++----
 test/zdtm/.gitignore                           |  1 +
 test/zdtm/live/static/Makefile                 |  1 +
 test/zdtm/live/static/remap_dead_pid.c         |  5 +-
 test/zdtm/live/static/remap_dead_pid_root.c    | 72 ++++++++++++++++++++++++++
 test/zdtm/live/static/remap_dead_pid_root.desc |  1 +
 6 files changed, 104 insertions(+), 10 deletions(-)
 create mode 100644 test/zdtm/live/static/remap_dead_pid_root.c
 create mode 100644 test/zdtm/live/static/remap_dead_pid_root.desc

diff --git a/files-reg.c b/files-reg.c
index 0f89be5..ea66af0 100644
--- a/files-reg.c
+++ b/files-reg.c
@@ -887,16 +887,32 @@ static int check_path_remap(struct fd_link *link, const struct fd_parms *parms,
 			return -1;
 		pid = strtol(start + 1, &end, 10);
 
-		/* if we didn't find another /, this path something
-		 * like ./proc/kmsg, which we shouldn't mess with. */
-		if (*end == '/') {
-			*end = 0;
-			ret = access(rpath, F_OK);
-			*end = '/';
-
-			if (ret) {
+		/* If strtol didn't convert anything, then we are looking at
+		 * something like /proc/kmsg, which we shouldn't mess with.
+		 * Anything under /proc/<pid> (including that directory itself)
+		 * can be c/r'd with a dead pid remap, so let's allow all such
+		 * cases.
+		 */
+		if (pid != 0) {
+			bool is_dead = strip_deleted(link);
+
+			/* /proc/<pid> will be "/proc/1 (deleted)" when it is
+			 * dead, but a path like /proc/1/mountinfo won't have
+			 * the suffix, since it isn't actually deleted (still
+			 * exists, but the parent dir is deleted). So, if we
+			 * have a path like /proc/1/mountinfo, test if /proc/1
+			 * exists instead, since this is what CRIU will need to
+			 * open on restore.
+			 */
+			if (!is_dead) {
+				*end = 0;
+				is_dead = access(rpath, F_OK);
+				*end = '/';
+			}
+
+			if (is_dead) {
 				pr_info("Dumping dead process remap of %d\n", pid);
-				return dump_dead_process_remap(pid, rpath + 1, plen - 1, ost, lfd, id, nsid);
+				return dump_dead_process_remap(pid, rpath + 1, link->len - 1, ost, lfd, id, nsid);
 			}
 		}
 
diff --git a/test/zdtm/.gitignore b/test/zdtm/.gitignore
index 3cdfa1a..5963609 100644
--- a/test/zdtm/.gitignore
+++ b/test/zdtm/.gitignore
@@ -106,6 +106,7 @@
 /live/static/pty03
 /live/static/pty04
 /live/static/remap_dead_pid
+/live/static/remap_dead_pid_root
 /live/static/rlimits00
 /live/static/rmdir_open
 /live/static/rtc
diff --git a/test/zdtm/live/static/Makefile b/test/zdtm/live/static/Makefile
index 42eea67..689b1e7 100644
--- a/test/zdtm/live/static/Makefile
+++ b/test/zdtm/live/static/Makefile
@@ -132,6 +132,7 @@ TST_NOFILE	=				\
 		dumpable01			\
 		dumpable02			\
 		remap_dead_pid			\
+		remap_dead_pid_root			\
 		aio00				\
 		fd				\
 		apparmor				\
diff --git a/test/zdtm/live/static/remap_dead_pid.c b/test/zdtm/live/static/remap_dead_pid.c
index c16832d..00fe3fd 100644
--- a/test/zdtm/live/static/remap_dead_pid.c
+++ b/test/zdtm/live/static/remap_dead_pid.c
@@ -41,8 +41,11 @@ int main(int argc, char **argv)
 		pid_t result;
 
 		sprintf(path, "/proc/%d/mountinfo", pid);
-
 		fd = open(path, O_RDONLY);
+		if (fd < 0) {
+			fail("failed to open fd");
+			return -1;
+		}
 
 		/* no matter what, we should kill the child */
 		kill(pid, SIGKILL);
diff --git a/test/zdtm/live/static/remap_dead_pid_root.c b/test/zdtm/live/static/remap_dead_pid_root.c
new file mode 100644
index 0000000..2a31412
--- /dev/null
+++ b/test/zdtm/live/static/remap_dead_pid_root.c
@@ -0,0 +1,72 @@
+#define _GNU_SOURCE
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <signal.h>
+#include <stdio.h>
+#include <limits.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include "zdtmtst.h"
+
+#ifndef CLONE_NEWNS
+#define CLONE_NEWNS     0x00020000
+#endif
+
+const char *test_doc	= "Check that dead pid's /proc entries are remapped correctly";
+const char *test_author	= "Tycho Andersen <tycho.andersen at canonical.com>";
+
+int main(int argc, char **argv)
+{
+	pid_t pid;
+
+	test_init(argc, argv);
+
+	pid = fork();
+	if (pid < 0) {
+		fail("fork() failed");
+		return -1;
+	}
+
+	if (pid == 0) {
+		test_msg("child is %d\n", pid);
+		/* Child process just sleeps until it is killed. All we need
+		 * here is a process to open the mountinfo of. */
+		while(1)
+			sleep(10);
+	} else {
+		int fd, ret;
+		char path[PATH_MAX];
+		pid_t result;
+
+		sprintf(path, "/proc/%d", pid);
+		fd = open(path, O_RDONLY);
+		if (fd < 0) {
+			fail("failed to open fd");
+			return -1;
+		}
+
+		/* no matter what, we should kill the child */
+		kill(pid, SIGKILL);
+		result = waitpid(pid, NULL, 0);
+		if (result < 0) {
+			fail("failed waitpid()");
+			return -1;
+		}
+
+		test_daemon();
+		test_waitsig();
+
+		ret = fcntl(fd, F_GETFD);
+		close(fd);
+
+		if (ret) {
+			fail("bad fd after restore");
+			return -1;
+		}
+	}
+
+	pass();
+	return 0;
+}
diff --git a/test/zdtm/live/static/remap_dead_pid_root.desc b/test/zdtm/live/static/remap_dead_pid_root.desc
new file mode 100644
index 0000000..63df42a
--- /dev/null
+++ b/test/zdtm/live/static/remap_dead_pid_root.desc
@@ -0,0 +1 @@
+{'flavor': 'h'}
-- 
2.5.0