From d4c02f2eb181197a70234b47be73f21824ff5ce5 Mon Sep 17 00:00:00 2001 From: Dmitry Safonov Date: Fri, 25 Nov 2016 15:51:00 +0300 Subject: [PATCH] compel: kill self-unmap in parasite 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? travis-ci: success for compel: kill self-unmap in parasite Cc: Cyrill Gorcunov Cc: Pavel Emelyanov Cc: Andrei Vagin Signed-off-by: Dmitry Safonov Acked-by: Andrei Vagin Signed-off-by: Pavel Emelyanov Signed-off-by: Andrei Vagin --- 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 3d9091159..f25ca89eb 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 7fb3db8b8..a9553c8d0 100644 --- a/compel/plugins/std/infect.c +++ b/compel/plugins/std/infect.c @@ -140,20 +140,6 @@ out: return 0; } -static noinline int unmap_itself(void *data) -{ - struct parasite_unmap_args *args = data; - - sys_munmap(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; @@ -202,12 +188,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 6674dc0f6..adef4db9e 100644 --- a/compel/src/lib/infect.c +++ b/compel/src/lib/infect.c @@ -1292,34 +1292,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 = 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;