mirror of
https://github.com/checkpoint-restore/criu.git
synced 2026-01-23 02:14:37 +00:00
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 <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@virtuozzo.com>
Cc: Andrei Vagin <avagin@virtuozzo.com>
Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
Acked-by: Andrei Vagin <avagin@virtuozzo.com>
Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
This commit is contained in:
parent
483f3e886b
commit
d4c02f2eb1
3 changed files with 10 additions and 42 deletions
|
|
@ -22,7 +22,6 @@ enum {
|
|||
PARASITE_CMD_ACK,
|
||||
|
||||
PARASITE_CMD_INIT_DAEMON,
|
||||
PARASITE_CMD_UNMAP,
|
||||
|
||||
/*
|
||||
* This must be greater than INITs.
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue