[CRIU] [PATCHv2] dump: preserve the dumpable flag on criu dump/restore

[Andrew Vagin]

On Tue, May 13, 2014 at 09:00:16AM -0700, Filipe Brandenburger wrote:
> Preserve the dumpable flag, which affects whether a core dump will be
> generated, but also affects the ownership of the virtual files under
> /proc/$pid after restoring a process.
> 
> Tested: Restored a process with a criu including this patch and looked
> at /proc/$pid to confirm that the virtual files were no longer all owned
> by root:root.
> 
> zdtm tests pass except for cow01 which seems to be broken.
> (see https://bugzilla.openvz.org/show_bug.cgi?id=2967 for details.)
> 
> This patch fixes https://bugzilla.openvz.org/show_bug.cgi?id=2968
> 
> Signed-off-by: Filipe Brandenburger <filbranden at google.com>
> Change-Id: I8c386508448a84368a86666f2d7500b252a78bbf
> ---
>  cr-dump.c          |  3 +++
>  include/parasite.h |  2 ++
>  include/prctl.h    |  6 ++++++
>  pie/parasite.c     |  1 +
>  pie/restorer.c     | 21 +++++++++++++++++++++
>  protobuf/mm.proto  |  2 ++
>  6 files changed, 35 insertions(+)
> 
> diff --git a/cr-dump.c b/cr-dump.c
> index a4100c1c40f1..9683a4ef5faf 100644
> --- a/cr-dump.c
> +++ b/cr-dump.c
> @@ -455,6 +455,9 @@ static int dump_task_mm(pid_t pid, const struct proc_pid_stat *stat,
>  
>  	mme.mm_brk = misc->brk;
>  
> +	mme.dumpable = misc->dumpable;
> +	mme.has_dumpable = true;
> +
>  	mme.n_mm_saved_auxv = AT_VECTOR_SIZE;
>  	mme.mm_saved_auxv = xmalloc(pb_repeated_size(&mme, mm_saved_auxv));
>  	if (!mme.mm_saved_auxv)
> diff --git a/include/parasite.h b/include/parasite.h
> index dd14da57c07e..9041ac842276 100644
> --- a/include/parasite.h
> +++ b/include/parasite.h
> @@ -157,6 +157,8 @@ struct parasite_dump_misc {
>  
>  	struct parasite_dump_thread	ti;
>  	u64 cycles;
> +
> +	int dumpable;
>  };
>  
>  #define PARASITE_MAX_GROUPS	(PAGE_SIZE / sizeof(unsigned int) - 2 * sizeof(unsigned))
> diff --git a/include/prctl.h b/include/prctl.h
> index 79fcb3529e26..9815c9f3ac3d 100644
> --- a/include/prctl.h
> +++ b/include/prctl.h
> @@ -16,6 +16,12 @@
>  #ifndef PR_SET_SECUREBITS
>  # define PR_SET_SECUREBITS	28
>  #endif
> +#ifndef PR_GET_DUMPABLE
> +# define PR_GET_DUMPABLE	3
> +#endif
> +#ifndef PR_SET_DUMPABLE
> +# define PR_SET_DUMPABLE	4
> +#endif
>  
>  #ifndef PR_SET_MM
>  #define PR_SET_MM		35
> diff --git a/pie/parasite.c b/pie/parasite.c
> index 5f13191fdad8..0c30603610ec 100644
> --- a/pie/parasite.c
> +++ b/pie/parasite.c
> @@ -163,6 +163,7 @@ static int dump_misc(struct parasite_dump_misc *args)
>  	args->umask = sys_umask(0);
>  	sys_umask(args->umask); /* never fails */
>  	args->cycles = arch_read_cycles();
> +	args->dumpable = sys_prctl(PR_GET_DUMPABLE, 0, 0, 0, 0);
>  
>  	return dump_thread_common(&args->ti);
>  }
> diff --git a/pie/restorer.c b/pie/restorer.c
> index ac8013971029..c8bf073f20f1 100644
> --- a/pie/restorer.c
> +++ b/pie/restorer.c
> @@ -30,6 +30,7 @@
>  #include "restorer.h"
>  
>  #include "protobuf/creds.pb-c.h"
> +#include "protobuf/mm.pb-c.h"
>  
>  #include "asm/restorer.h"
>  #include "asm/cycles.h"
> @@ -188,6 +189,21 @@ static int restore_creds(CredsEntry *ce)
>  	return 0;
>  }
>  
> +static int restore_dumpable_flag(MmEntry *mme)
> +{
> +	int ret;
> +
> +	if (mme->has_dumpable) {
> +		ret = sys_prctl(PR_SET_DUMPABLE, mme->dumpable, 0, 0, 0);

prctl((PR_SET_DUMPABLE, 2) always fails. Filipe, do you want to
investigate how to handle this situation?

Thanks.

man 2 prctl:
...
In kernels  up  to  and  including  2.6.12,  arg2  must be either 0
(process is not dumpable) or 1 (process is  dumpable).   Between
kernels 2.6.13 and 2.6.17, the value 2 was also permitted, which
caused any binary which normally  would  not  be  dumped  to  be
dumped readable by root only; for security reasons, this feature
has   been   removed.
...

> +		if (ret) {
> +			pr_err("Unable to set PR_SET_DUMPABLE: %d\n", ret);
> +			return -1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static void restore_sched_info(struct rst_sched_param *p)
>  {
>  	struct sched_param parm;
> @@ -296,6 +312,10 @@ long __export_restore_thread(struct thread_restore_args *args)
>  	if (ret)
>  		goto core_restore_end;
>  
> +	ret = restore_dumpable_flag(&args->ta->mm);
> +	if (ret)
> +		goto core_restore_end;
> +
>  	pr_info("%ld: Restored\n", sys_gettid());
>  
>  	restore_finish_stage(CR_STATE_RESTORE);
> @@ -930,6 +950,7 @@ long __export_restore_task(struct task_restore_args *args)
>  	 */
>  
>  	ret = restore_creds(&args->creds);
> +	ret = ret || restore_dumpable_flag(&args->mm);
>  
>  	futex_set_and_wake(&thread_inprogress, args->nr_threads);
>  
> diff --git a/protobuf/mm.proto b/protobuf/mm.proto
> index b809061afdd1..1556b602d588 100644
> --- a/protobuf/mm.proto
> +++ b/protobuf/mm.proto
> @@ -17,4 +17,6 @@ message mm_entry {
>  	repeated uint64	mm_saved_auxv	= 13;
>  
>  	repeated vma_entry vmas		= 14;
> +
> +	optional int32	dumpable	= 15;
>  }
> -- 
> 1.9.1.423.g4596e3a
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu