[CRIU] [PATCH 3/3] criu: Restore anonymized images(file paths)

[Pavel Emelianov]

On 23.09.2019 13:30, Harshavardhan Unnibhavi wrote:
> 
> 
> On Mon, Sep 23, 2019 at 3:39 PM Pavel Emelianov <xemul at virtuozzo.com <mailto:xemul at virtuozzo.com>> wrote:
> 
> 
> 
>     On 20.09.2019 11:15, Harshavardhan Unnibhavi wrote:
>      > This commit restores anonymized images containing their file paths
>      > maximized.
>      >
>      > This commit introduces the following:
>      >    - anon option for the restore action.
>      >
>      > This commit modifies the following:
>      >    - pretend anonymized files in question are opened.
>      >
>      > Resolve Issue #360.
>      >
>      > Signed-off-by: Harshavardhan Unnibhavi <hvubfoss at gmail.com <mailto:hvubfoss at gmail.com>>
>      > ---
>      >   criu/config.c             |  5 +++++
>      >   criu/cr-restore.c         |  5 +++++
>      >   criu/files-reg.c          | 12 ++++++++++--
>      >   criu/files.c              |  4 ++--
>      >   criu/include/cr_options.h |  1 +
>      >   criu/include/restorer.h   |  1 +
>      >   criu/pie/restorer.c       |  4 ++--
>      >   7 files changed, 26 insertions(+), 6 deletions(-)
>      >
>      > diff --git a/criu/config.c b/criu/config.c
>      > index c9332203..7881f70d 100644
>      > --- a/criu/config.c
>      > +++ b/criu/config.c
>      > @@ -276,6 +276,7 @@ void init_opts(void)
>      >       opts.empty_ns = 0;
>      >       opts.status_fd = -1;
>      >       opts.log_level = DEFAULT_LOGLEVEL;
>      > +     opts.anonymize = false;
>      >   }
>      >
>      >   bool deprecated_ok(char *what)
>      > @@ -455,6 +456,7 @@ int parse_options(int argc, char **argv, bool *usage_error,
>      >               { "root",                       required_argument,      0, 'r'  },
>      >               { USK_EXT_PARAM,                optional_argument,      0, 'x'  },
>      >               { "help",                       no_argument,            0, 'h'  },
>      > +             { "anon",                       no_argument,            0, 'a'  },
>      >               BOOL_OPT(SK_EST_PARAM, &opts.tcp_established_ok),
>      >               { "close",                      required_argument,      0, 1043 },
>      >               BOOL_OPT("log-pid", &opts.log_file_per_pid),
>      > @@ -575,6 +577,9 @@ int parse_options(int argc, char **argv, bool *usage_error,
>      >                       continue;
>      >
>      >               switch (opt) {
>      > +             case 'a':
>      > +                     opts.anonymize = true;
>      > +                     break;
>      >               case 's':
>      >                       opts.final_state = TASK_STOPPED;
>      >                       break;
>      > diff --git a/criu/cr-restore.c b/criu/cr-restore.c
>      > index de0b2cb4..05896ab0 100644
>      > --- a/criu/cr-restore.c
>      > +++ b/criu/cr-restore.c
>      > @@ -882,6 +882,9 @@ static int restore_one_alive_task(int pid, CoreEntry *core)
>      >
>      >       memzero(ta, args_len);
>      >
>      > +     if(opts.anonymize)
>      > +             ta->anonymize = true;
>      > +
>      >       if (prepare_fds(current))
>      >               return -1;
>      >
>      > @@ -2268,6 +2271,8 @@ skip_ns_bouncing:
>      >               goto out_kill;
>      >
>      >       pr_info("Restore finished successfully. Resuming tasks.\n");
>      > +     if(opts.anonymize)
>      > +             return 0;
> 
>     Presumably you should abort the stage so that "restored" tasks die?
> 
> How would I do this? Using sigreturn( rst_sigreturn(new_sp, rt_sigframe) ) or a sys_exit()?

         __restore_switch_stage(CR_STATE_FAIL);

>     Also, a message about "aborting restore due to --anon given" is also needed.
> 
> Sure.
> 
> 
>      >       __restore_switch_stage(CR_STATE_COMPLETE);
>      >
>      >       if (ret == 0)
>      > diff --git a/criu/files-reg.c b/criu/files-reg.c
>      > index 1b51d108..a3a2396d 100644
>      > --- a/criu/files-reg.c
>      > +++ b/criu/files-reg.c
>      > @@ -1745,14 +1745,14 @@ ext:
>      >                       return -1;
>      >               }
>      >
>      > -             if (rfi->rfe->has_size && (st.st_size != rfi->rfe->size)) {
>      > +             if (rfi->rfe->has_size && (st.st_size != rfi->rfe->size) && (!opts.anonymize)) {
>      >                       pr_err("File %s has bad size %"PRIu64" (expect %"PRIu64")\n",
>      >                                       rfi->path, st.st_size,
>      >                                       rfi->rfe->size);
>      >                       return -1;
>      >               }
>      >
>      > -             if (rfi->rfe->has_mode && (st.st_mode != rfi->rfe->mode)) {
>      > +             if (rfi->rfe->has_mode && (st.st_mode != rfi->rfe->mode) && (!opts.anonymize)) {
>      >                       pr_err("File %s has bad mode 0%o (expect 0%o)\n",
>      >                              rfi->path, (int)st.st_mode,
>      >                              rfi->rfe->mode);
> 
>     The whole block with stat()-ing and size/mode checking can be skipped if opts.anonymize.
> 
>      > @@ -1793,6 +1793,14 @@ int do_open_reg_noseek_flags(int ns_root_fd, struct reg_file_info *rfi, void *ar
>      >       flags &= ~O_TMPFILE;
>      >
>      >       fd = openat(ns_root_fd, rfi->path, flags);
>      > +     if(opts.anonymize){
> 
>     Plz, check the patch with checkpatch.pl <http://checkpatch.pl> for coding style.
> 
>      > +             fd = openat(ns_root_fd, "/dev/zero", flags);
> 
>     Erm... The original fd has been opened, hasn't it? I think it's better to fix
>     the do_open_reg_noseek() to open /dev/zero in case of opts.anonymize, not this helper.
> 
> Yes this makes more sense, I will change it.

Awesome!

-- Pavel