Skip to content

Commit

Permalink
Sanitize TASK_ values
Browse files Browse the repository at this point in the history
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).

travis-ci: success for compel uapi cleanups (rev2)
Signed-off-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Pavel Emelyanov <[email protected]>
  • Loading branch information
kolyshkin authored and xemul committed Feb 10, 2017
1 parent d5cd776 commit 2933877
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 27 deletions.
10 changes: 1 addition & 9 deletions compel/include/uapi/infect.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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;

Expand Down
19 changes: 19 additions & 0 deletions compel/include/uapi/task-state.h
Original file line number Diff line number Diff line change
@@ -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__ */
20 changes: 10 additions & 10 deletions compel/src/lib/infect.c
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,9 @@ int compel_wait_task(int pid, int ppid,
}

if (ret < 0)
return TASK_ZOMBIE;
return COMPEL_TASK_ZOMBIE;
else
return TASK_DEAD;
return COMPEL_TASK_DEAD;
}

if ((ppid != -1) && (ss->ppid != ppid)) {
Expand Down Expand Up @@ -289,11 +289,11 @@ int compel_wait_task(int pid, int ppid,
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;
Expand All @@ -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);
Expand Down
26 changes: 18 additions & 8 deletions criu/include/pid.h
Original file line number Diff line number Diff line change
@@ -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;
/*
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions criu/pstree.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 2933877

Please sign in to comment.