[CRIU] [RFC] compel: kill self-unmap in parasite

[Dmitry Safonov]

Why should we have self-unmapping code in parasite?
It looks like, we can drop this code using simple sys_unmap()
injection (like that I did for `criu exec` action and for cases where we
failed to insert parasite by some reason, but still need to unmap remotes).

It's an RFC, so just a suggestion - maybe I miss something you have in
mind - please, describe that/those things.

My motivation is:
- less code, defined commands for PIE, one BUG() less, one jump to PIE less
- I'm making one 64-bit parasite on x86 instead of two 32 and 64 bit.
  It works (branch 32-one-parasite) with long-jump in the beginning to
  64-bit code from 32-bit task.
  On parasite curing it sig-returns from 64-bit parasite to 32-bit task,
  this point we're trapping in CRIU. After that we command parasite to
  unmap itself, so it long-jumps again to parasite 64-bit code, unmaps,
  we caught task after sys_unmap and the task is with 64-bit CS.
  We can't set 32-bit registers after this - kernel checks that
  registers set is the same on PTRACE_SETREGSET:
> static int ptrace_regset(struct task_struct *task, int req, unsigned int type,
>                        struct iovec *kiov)
...
>       if (!regset || (kiov->iov_len % regset->size) != 0)
>               return -EINVAL;
  So, to return again to 32-bit task I need sigreturn() again or add
  long-jump with 32-bit CS.
  I've disable that for 32-bit testing with (in compel_cure_remote):
-       if (ctl->addr_cmd) {
+       if (ctl->addr_cmd && user_regs_native(&ctl->orig.regs)) {
  And it works. It also works for native tasks, so why should we keep it?

Cc: Cyrill Gorcunov <gorcunov at openvz.org>
Cc: Pavel Emelyanov <xemul at virtuozzo.com>
Cc: Andrei Vagin <avagin at virtuozzo.com>
Signed-off-by: Dmitry Safonov <dsafonov at virtuozzo.com>
---
 compel/include/rpc-pie-priv.h |  1 -
 compel/plugins/std/infect.c   | 20 +-------------------
 compel/src/lib/infect.c       | 31 +++++++++----------------------
 3 files changed, 10 insertions(+), 42 deletions(-)

diff --git a/compel/include/rpc-pie-priv.h b/compel/include/rpc-pie-priv.h
index 3d9091159737..f25ca89eb344 100644
--- a/compel/include/rpc-pie-priv.h
+++ b/compel/include/rpc-pie-priv.h
@@ -22,7 +22,6 @@ enum {
 	PARASITE_CMD_ACK,
 
 	PARASITE_CMD_INIT_DAEMON,
-	PARASITE_CMD_UNMAP,
 
 	/*
 	 * This must be greater than INITs.
diff --git a/compel/plugins/std/infect.c b/compel/plugins/std/infect.c
index 4d06814516c4..e0cdb644a27f 100644
--- a/compel/plugins/std/infect.c
+++ b/compel/plugins/std/infect.c
@@ -141,20 +141,6 @@ out:
 	return 0;
 }
 
-static noinline int unmap_itself(void *data)
-{
-	struct parasite_unmap_args *args = data;
-
-	sys_munmap((void*)(uintptr_t)args->parasite_start, args->parasite_len);
-	/*
-	 * This call to sys_munmap must never return. Instead, the controlling
-	 * process must trap us on the exit from munmap.
-	 */
-
-	BUG();
-	return -1;
-}
-
 static noinline __used int parasite_init_daemon(void *data)
 {
 	struct parasite_init_args *args = data;
@@ -203,12 +189,8 @@ int __used __parasite_entry parasite_service(unsigned int cmd, void *args)
 {
 	pr_info("Parasite cmd %d/%x process\n", cmd, cmd);
 
-	switch (cmd) {
-	case PARASITE_CMD_INIT_DAEMON:
+	if (cmd == PARASITE_CMD_INIT_DAEMON)
 		return parasite_init_daemon(args);
-	case PARASITE_CMD_UNMAP:
-		return unmap_itself(args);
-	}
 
 	return parasite_trap_cmd(cmd, args);
 }
diff --git a/compel/src/lib/infect.c b/compel/src/lib/infect.c
index b85f36ec6ecb..df4a3201e2af 100644
--- a/compel/src/lib/infect.c
+++ b/compel/src/lib/infect.c
@@ -1286,34 +1286,21 @@ int compel_stop_daemon(struct parasite_ctl *ctl)
 
 int compel_cure_remote(struct parasite_ctl *ctl)
 {
+	unsigned long ret;
+
 	if (compel_stop_daemon(ctl))
 		return -1;
 
 	if (!ctl->remote_map)
 		return 0;
 
-	/* Unseizing task with parasite -- it does it himself */
-	if (ctl->addr_cmd) {
-		struct parasite_unmap_args *args;
-
-		*ctl->addr_cmd = PARASITE_CMD_UNMAP;
-
-		args = compel_parasite_args(ctl, struct parasite_unmap_args);
-		args->parasite_start = (uintptr_t)ctl->remote_map;
-		args->parasite_len = ctl->map_length;
-		if (compel_unmap(ctl, ctl->parasite_ip))
-			return -1;
-	} else {
-		unsigned long ret;
-
-		compel_syscall(ctl, __NR(munmap, !compel_mode_native(ctl)), &ret,
-				(unsigned long)ctl->remote_map, ctl->map_length,
-				0, 0, 0, 0);
-		if (ret) {
-			pr_err("munmap for remote map %p, %lu returned %lu\n",
-					ctl->remote_map, ctl->map_length, ret);
-			return -1;
-		}
+	compel_syscall(ctl, __NR(munmap, !compel_mode_native(ctl)), &ret,
+			(unsigned long)ctl->remote_map, ctl->map_length,
+			0, 0, 0, 0);
+	if (ret) {
+		pr_err("munmap for remote map %p, %lu returned %lu\n",
+				ctl->remote_map, ctl->map_length, ret);
+		return -1;
 	}
 
 	return 0;
-- 
2.10.2