Skip to content

Commit

Permalink
cr-restore.c: restore SUD settings
Browse files Browse the repository at this point in the history
Add hooks for restoring SUD settings throughout cr-restore.c.

The implementation is a little unorthodox. Unlike seccomp, SUD isn't suspended while a task is under ptrace. So the parasite code cannot restore the SUD settings because SIGSYS may be triggered when the restorer blob is unmapped.
Instead, we opt to reopen the per-core data right before PTRACE_DETACH, and restore the SUD settings then. This way we don't risk triggering SIGSYS via a remote syscall.

Signed-off-by: Svetly Todorov <[email protected]>
Signed-off-by: Gregory Price <[email protected]>
  • Loading branch information
svetly-todorov committed Jun 21, 2024
1 parent eb59348 commit d4b0cc1
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 4 deletions.
24 changes: 20 additions & 4 deletions criu/cr-restore.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
#include "aio.h"
#include "lsm.h"
#include "seccomp.h"
#include "sud.h"
#include "fault-injection.h"
#include "sk-queue.h"
#include "sigframe.h"
Expand Down Expand Up @@ -319,6 +320,9 @@ static int root_prepare_shared(void)

if (seccomp_read_image())
return -1;

if (sud_read_image())
return -1;

if (collect_images(cinfos, ARRAY_SIZE(cinfos)))
return -1;
Expand Down Expand Up @@ -804,13 +808,17 @@ static int open_cores(int pid, CoreEntry *leader_core)
* Otherwise any criu code which might use same syscall
* if present inside a filter chain would take filter
* action and might break restore procedure.
*
* Save the syscall user dispatch setting as well.
*/
for (i = 0; i < current->nr_threads; i++) {
ThreadCoreEntry *thread_core = cores[i]->thread_core;
if (thread_core->seccomp_mode != SECCOMP_MODE_DISABLED) {

if (thread_core->seccomp_mode != SECCOMP_MODE_DISABLED)
rsti(current)->has_seccomp = true;
break;
}

if (thread_core->sud_mode != SYS_DISPATCH_OFF)
rsti(current)->has_sud = true;
}

for (i = 0; i < current->nr_threads; i++) {
Expand Down Expand Up @@ -1377,13 +1385,14 @@ static inline int fork_with_pid(struct pstree_item *item)
}

/*
* By default we assume that seccomp is not
* By default we assume that neither seccomp nor SUD is
* used at all (especially on dead task). Later
* we will walk over all threads and check in
* details if filter is present setting up
* this flag as appropriate.
*/
rsti(item)->has_seccomp = false;
rsti(item)->has_sud = false;

if (unlikely(item == root_item))
maybe_clone_parent(item, &ca);
Expand Down Expand Up @@ -2016,6 +2025,9 @@ static int attach_to_tasks(bool root_seized)
*/
if (rsti(item)->has_seccomp && ptrace_suspend_seccomp(pid) < 0)
pr_err("failed to suspend seccomp, restore will probably fail...\n");
/* Suspend SUD for similar reasons. */
if (rsti(item)->has_sud && ptrace_suspend_sud(pid) < 0)
pr_err("failed to suspend sud, restore may fail...\n");

if (ptrace(PTRACE_CONT, pid, NULL, NULL)) {
pr_perror("Unable to resume %d", pid);

Check notice

Code scanning / CodeQL

FIXME comment Note

FIXME comment: -- currently we assume that all the tasks live [...]
Expand Down Expand Up @@ -2166,6 +2178,10 @@ static int finalize_restore_detach(void)
pr_perror("Restoring regs for %d failed", pid);
return -1;
}
if (restore_sud_per_core(pid)) {
pr_perror("Restoring syscall dispatch state for %d failed", pid);
return -1;
}
if (ptrace(PTRACE_DETACH, pid, NULL, 0)) {
pr_perror("Unable to detach %d", pid);
return -1;
Expand Down
4 changes: 4 additions & 0 deletions criu/include/rst_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ struct rst_info {
* are bound to group leader and we need to use tsync flag.
*/
bool has_old_seccomp_filter;
/*
* We set a similar flag for syscall user dispatch.
*/
bool has_sud;

struct rst_rseq *rseqe;

Expand Down
102 changes: 102 additions & 0 deletions criu/sud.c
Original file line number Diff line number Diff line change
Expand Up @@ -165,3 +165,105 @@ int dump_sud(void)

return ret;
}

/*
* Read SUD settings from the checkpoint image.
*
* Since the code here rips from seccomp, all of the settings
* for a process tree will be stored in one image entry.
*
* Compare this to something like file-lock.c:collect_one_file_lock,
* which reads FileLockEntries sequentially from protobuf and daisy-
* chains them into file_lock_list.
*/
int sud_read_image(void)
{
struct cr_img *img;
int ret;

if (sud_img_entry)
return 0;

img = open_image(CR_FD_SYS_DISPATCH, O_RSTR);
if (!img)
return -1;

ret = pb_read_one_eof(img, &sud_img_entry, PB_SYS_DISPATCH);
close_image(img);
if (ret <= 0)
return 0; /* all threads had SYS_DISPATCH_OFF */

BUG_ON(!sud_img_entry);

return 0;
}

static int open_core(int pid, CoreEntry **pcore)
{
struct cr_img *img;
int ret;

img = open_image(CR_FD_CORE, O_RSTR, pid);
if (!img) {
pr_err("Can't open core data for %d\n", pid);
return -1;
}
ret = pb_read_one(img, pcore, PB_CORE);
close_image(img);

return ret <= 0 ? -1 : 0;
}

/*
* Restore SUD directly from the image data, to
* avoid disable/enable shuffling in the restorer blob.
*
* I believe this is OK because this is called in
* cr-restore.c:finalize_restore_detach(), which happens
* a few lines before close_image_dir(), which closes the
* image streamer.
*/
int restore_sud_per_core(pid_t tid_real)
{
sud_config_t config;
SysDispatchSetting *ss;
CoreEntry *core;
ThreadCoreEntry *tc;
int ret = 0;

sud_read_image();

core = xmalloc(sizeof(*core));
if (open_core(tid_real, &core) < 0) {
pr_perror("Cannot open core for tid %d", tid_real);
return -1;
}

tc = core->thread_core;

if (!tc->has_sud_mode)
goto cleanup_exit;

if (tc->sud_mode == SYS_DISPATCH_OFF)
goto cleanup_exit;

if (tc->sud_setting >= sud_img_entry->n_settings) {
ret = -1;
pr_err("Corrupted sud setting index on tid %d (%u > %zu)\n", tid_real,
tc->sud_setting, sud_img_entry->n_settings);
goto cleanup_exit;
}

ss = sud_img_entry->settings[tc->sud_setting];

config.mode = SYS_DISPATCH_ON;
config.selector = ss->selector;
config.offset = ss->offset;
config.len = ss->len;

ret = ptrace_set_sud(tid_real, &config);

cleanup_exit:
xfree(core);
return ret;
}

0 comments on commit d4b0cc1

Please sign in to comment.