[CRIU] [PATCH v2] Sanitize TASK_ values

Kir Kolyshkin kir at openvz.org
Thu Feb 9 14:03:55 PST 2017


First, TASK_* defines provided by compel should be prefixed
with COMPEL_. The complication is, same constants are also used
by CRIU, some are even writted into images (meaning we should
not change their values).

One way to solve this would be to untie compel values from CRIU ones,
using some mapping between the two sets when needed (i.e. in calls to
compel_wait_task() and compel_resume_task()).

Fortunately, we can avoid implementing this mapping by separating
the ranges used by compel and criu. With this patch, compel is using
values in range 0x01..0x7f, and criu is reusing those, plus adding
more values in range 0x80..0xff for its own purposes.

Note tha the values that are used inside images are not changed
(as, luckily, they were all used by compel).

Signed-off-by: Kir Kolyshkin <kir at openvz.org>
---
 compel/include/uapi/infect.h     | 10 +---------
 compel/include/uapi/task-state.h | 19 +++++++++++++++++++
 compel/src/lib/infect.c          | 20 ++++++++++----------
 criu/include/pid.h               | 26 ++++++++++++++++++--------
 criu/pstree.c                    |  1 +
 5 files changed, 49 insertions(+), 27 deletions(-)
 create mode 100644 compel/include/uapi/task-state.h

diff --git a/compel/include/uapi/infect.h b/compel/include/uapi/infect.h
index 5c47555..da0ca3a 100644
--- a/compel/include/uapi/infect.h
+++ b/compel/include/uapi/infect.h
@@ -7,6 +7,7 @@
 #include <compel/asm/infect-types.h>
 #include <compel/ksigset.h>
 #include <compel/handle-elf.h>
+#include <compel/task-state.h>
 
 #include "common/compiler.h"
 
@@ -29,15 +30,6 @@ extern int compel_wait_task(int pid, int ppid,
 extern int compel_stop_task(int pid);
 extern int compel_resume_task(pid_t pid, int orig_state, int state);
 
-/*
- * FIXME -- these should be mapped to pid.h's
- */
-
-#define TASK_ALIVE		0x1
-#define TASK_DEAD		0x2
-#define TASK_STOPPED		0x3
-#define TASK_ZOMBIE		0x6
-
 struct parasite_ctl;
 struct parasite_thread_ctl;
 
diff --git a/compel/include/uapi/task-state.h b/compel/include/uapi/task-state.h
new file mode 100644
index 0000000..84a2a0b
--- /dev/null
+++ b/compel/include/uapi/task-state.h
@@ -0,0 +1,19 @@
+#ifndef __COMPEL_UAPI_TASK_STATE_H__
+#define __COMPEL_UAPI_TASK_STATE_H__
+
+/*
+ * Task state, as returned by compel_wait_task()
+ * and used in arguments to compel_resume_task().
+ */
+enum __compel_task_state
+{
+	COMPEL_TASK_ALIVE	= 0x01,
+	COMPEL_TASK_DEAD	= 0x02,
+	COMPEL_TASK_STOPPED	= 0x03,
+	COMPEL_TASK_ZOMBIE	= 0x06,
+	/* Don't ever change the above values, they are used by CRIU! */
+
+	COMPEL_TASK_MAX		= 0x7f
+};
+
+#endif /* __COMPEL_UAPI_TASK_STATE_H__ */
diff --git a/compel/src/lib/infect.c b/compel/src/lib/infect.c
index 345079b..fea7e6b 100644
--- a/compel/src/lib/infect.c
+++ b/compel/src/lib/infect.c
@@ -235,9 +235,9 @@ try_again:
 		}
 
 		if (ret < 0)
-			return TASK_ZOMBIE;
+			return COMPEL_TASK_ZOMBIE;
 		else
-			return TASK_DEAD;
+			return COMPEL_TASK_DEAD;
 	}
 
 	if ((ppid != -1) && (ss->ppid != ppid)) {
@@ -289,11 +289,11 @@ try_again:
 		if (skip_sigstop(pid, nr_sigstop))
 			goto err_stop;
 
-		return TASK_STOPPED;
+		return COMPEL_TASK_STOPPED;
 	}
 
 	if (si.si_signo == SIGTRAP)
-		return TASK_ALIVE;
+		return COMPEL_TASK_ALIVE;
 	else {
 		pr_err("SEIZE %d: unsupported stop signal %d\n", pid, si.si_signo);
 		goto err;
@@ -311,25 +311,25 @@ int compel_resume_task(pid_t pid, int orig_st, int st)
 {
 	pr_debug("\tUnseizing %d into %d\n", pid, st);
 
-	if (st == TASK_DEAD) {
+	if (st == COMPEL_TASK_DEAD) {
 		kill(pid, SIGKILL);
 		return 0;
-	} else if (st == TASK_STOPPED) {
+	} else if (st == COMPEL_TASK_STOPPED) {
 		/*
 		 * Task might have had STOP in queue. We detected such
-		 * guy as TASK_STOPPED, but cleared signal to run the
-		 * parasite code. hus after detach the task will become
+		 * guy as COMPEL_TASK_STOPPED, but cleared signal to run
+		 * the parasite code. Thus after detach the task will become
 		 * running. That said -- STOP everyone regardless of
 		 * the initial state.
 		 */
 		kill(pid, SIGSTOP);
-	} else if (st == TASK_ALIVE) {
+	} else if (st == COMPEL_TASK_ALIVE) {
 		/*
 		 * Same as in the comment above -- there might be a
 		 * task with STOP in queue that would get lost after
 		 * detach, so stop it again.
 		 */
-		if (orig_st == TASK_STOPPED)
+		if (orig_st == COMPEL_TASK_STOPPED)
 			kill(pid, SIGSTOP);
 	} else
 		pr_err("Unknown final state %d\n", st);
diff --git a/criu/include/pid.h b/criu/include/pid.h
index 9ac583f..81786ec 100644
--- a/criu/include/pid.h
+++ b/criu/include/pid.h
@@ -1,9 +1,27 @@
 #ifndef __CR_PID_H__
 #define __CR_PID_H__
 
+#include <compel/task-state.h>
 #include "stdbool.h"
 #include "rbtree.h"
 
+/*
+ * Task states, used in e.g. struct pid's state.
+ */
+enum __criu_task_state
+{
+	/* Values shared with compel */
+	TASK_ALIVE		= COMPEL_TASK_ALIVE,
+	TASK_DEAD		= COMPEL_TASK_DEAD,
+	TASK_STOPPED		= COMPEL_TASK_STOPPED,
+	TASK_ZOMBIE		= COMPEL_TASK_ZOMBIE,
+	/* Own internal states */
+	TASK_HELPER		= COMPEL_TASK_MAX + 1,
+	TASK_THREAD,
+	/* new values are to be added before this line */
+	TASK_UNDEF		= 0xff
+};
+
 struct pid {
 	struct pstree_item *item;
 	/*
@@ -26,14 +44,6 @@ struct pid {
 	} ns[1]; /* Must be at the end of struct pid */
 };
 
-#define TASK_UNDEF		0x0
-#define TASK_ALIVE		0x1
-#define TASK_DEAD		0x2
-#define TASK_STOPPED		0x3
-#define TASK_HELPER		0x4
-#define TASK_THREAD		0x5
-#define TASK_ZOMBIE		0x6
-
 /*
  * When we have to restore a shared resource, we mush select which
  * task should do it, and make other(s) wait for it. In order to
diff --git a/criu/pstree.c b/criu/pstree.c
index 833b3d0..75a7916 100644
--- a/criu/pstree.c
+++ b/criu/pstree.c
@@ -218,6 +218,7 @@ struct pstree_item *__alloc_pstree_item(bool rst)
 
 	item->pid->ns[0].virt = -1;
 	item->pid->real = -1;
+	item->pid->state = TASK_UNDEF;
 	item->born_sid = -1;
 	futex_init(&item->task_st);
 	item->pid->item = item;
-- 
2.9.3



More information about the CRIU mailing list