[CRIU] [PATCH] rst: Sanitize standard arrays remapping

Andrew Vagin avagin at odin.com
Tue Jun 9 08:17:08 PDT 2015


On Fri, Jun 05, 2015 at 07:13:27PM +0300, Pavel Emelyanov wrote:
> On restore we have several arrays of objects that get remapped
> into pie area and their number is also passed. Clean and shorten
> the remapping code a bit and bing their naming to common format.
>

Acked-by: Andrew Vagin <avagin at odin.com>
 
> Signed-off-by: Pavel Emelyanov <xemul at parallels.com>
> ---
>  cr-restore.c       | 45 +++++++++++++++++----------------------------
>  include/restorer.h | 50 ++++++++++++++++++++++++++++----------------------
>  pie/restorer.c     | 48 ++++++++++++++++++++++++------------------------
>  3 files changed, 69 insertions(+), 74 deletions(-)
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index aa00dc2..2db4fc5 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -2855,29 +2855,21 @@ static int sigreturn_restore(pid_t pid, CoreEntry *core)
>  	task_args->premmapped_addr = (unsigned long)rsti(current)->premmapped_addr;
>  	task_args->premmapped_len = rsti(current)->premmapped_len;
>  
> -	task_args->nr_vmas = vmas->nr;
> -	task_args->tgt_vmas = rst_mem_remap_ptr(tgt_vmas, RM_PRIVATE);
> -
> -	task_args->timer_n = posix_timers_nr;
> -	task_args->posix_timers = rst_mem_remap_ptr(posix_timers_cpos, RM_PRIVATE);
> -
> -	task_args->timerfd_n = rst_timerfd_nr;
> -	task_args->timerfd = rst_mem_remap_ptr(timerfd_mem_cpos, RM_PRIVATE);
> -
> -	task_args->siginfo_nr = siginfo_nr;
> -	task_args->siginfo = rst_mem_remap_ptr(siginfo_cpos, RM_PRIVATE);
> -
> -	task_args->tcp_socks_nr = rst_tcp_socks_nr;
> -	task_args->tcp_socks = rst_mem_remap_ptr(tcp_socks, RM_PRIVATE);
> -
> -	task_args->nr_rings = mm->n_aios;
> -	task_args->rings = rst_mem_remap_ptr(aio_rings, RM_PRIVATE);
> -
> -	task_args->n_helpers = n_helpers;
> -	if (n_helpers > 0)
> -		task_args->helpers = rst_mem_remap_ptr(helpers_pos, RM_PRIVATE);
> -	else
> -		task_args->helpers = NULL;
> +#define remap_array(name, nr, cpos)	do {				\
> +		task_args->name##_n = nr;				\
> +		task_args->name = rst_mem_remap_ptr(cpos, RM_PRIVATE);	\
> +	} while (0)
> +
> +	remap_array(vmas,	  vmas->nr, tgt_vmas);
> +	remap_array(posix_timers, posix_timers_nr, posix_timers_cpos);
> +	remap_array(timerfd,	  rst_timerfd_nr, timerfd_mem_cpos);
> +	remap_array(siginfo,	  siginfo_nr, siginfo_cpos);
> +	remap_array(tcp_socks,	  rst_tcp_socks_nr, tcp_socks);
> +	remap_array(rings,	  mm->n_aios, aio_rings);
> +	remap_array(rlims,	  rlims_nr, rlims_cpos);
> +	remap_array(helpers,	  n_helpers, helpers_pos);
> +
> +#undef remap_array
>  
>  	if (lsm) {
>  		task_args->proc_attr_current = open_proc_rw(PROC_SELF, "attr/current");
> @@ -2904,9 +2896,6 @@ static int sigreturn_restore(pid_t pid, CoreEntry *core)
>  
>  	strncpy(task_args->comm, core->tc->comm, sizeof(task_args->comm));
>  
> -	task_args->nr_rlim = rlims_nr;
> -	if (rlims_nr)
> -		task_args->rlims = rst_mem_remap_ptr(rlims_cpos, RM_PRIVATE);
>  
>  	/*
>  	 * Fill up per-thread data.
> @@ -2916,10 +2905,10 @@ static int sigreturn_restore(pid_t pid, CoreEntry *core)
>  		struct rt_sigframe *sigframe;
>  
>  		thread_args[i].pid = current->threads[i].virt;
> -		thread_args[i].siginfo_nr = siginfo_priv_nr[i];
> +		thread_args[i].siginfo_n = siginfo_priv_nr[i];
>  		thread_args[i].siginfo = rst_mem_remap_ptr(siginfo_cpos, RM_PRIVATE);
>  		thread_args[i].siginfo += siginfo_nr;
> -		siginfo_nr += thread_args[i].siginfo_nr;
> +		siginfo_nr += thread_args[i].siginfo_n;
>  
>  		/* skip self */
>  		if (thread_args[i].pid == pid) {
> diff --git a/include/restorer.h b/include/restorer.h
> index 34396e3..54f4166 100644
> --- a/include/restorer.h
> +++ b/include/restorer.h
> @@ -90,7 +90,8 @@ struct thread_restore_args {
>  	tls_t				tls;
>  
>  	siginfo_t			*siginfo;
> -	unsigned int			siginfo_nr;
> +	unsigned int			siginfo_n;
> +
>  	int				pdeath_sig;
>  } __aligned(64);
>  
> @@ -109,10 +110,33 @@ struct task_restore_args {
>  	struct task_entries		*task_entries;
>  	void				*rst_mem;
>  	unsigned long			rst_mem_size;
> -	VmaEntry			*tgt_vmas;
> +
> +	/* Below arrays get remapped from RM_PRIVATE in sigreturn_restore */
> +	VmaEntry			*vmas;
> +	unsigned int			vmas_n;
> +
> +	struct restore_posix_timer	*posix_timers;
> +	unsigned int			posix_timers_n;
> +
> +	struct restore_timerfd		*timerfd;
> +	unsigned int			timerfd_n;
> +
>  	siginfo_t			*siginfo;
> -	unsigned int			siginfo_nr;
> -	unsigned int			nr_vmas;
> +	unsigned int			siginfo_n;
> +
> +	struct rst_tcp_sock		*tcp_socks;
> +	unsigned int			tcp_socks_n;
> +
> +	struct rst_aio_ring		*rings;
> +	unsigned int			rings_n;
> +
> +	struct rlimit			*rlims;
> +	unsigned int			rlims_n;
> +
> +	pid_t				*helpers /* the TASK_HELPERS to wait on at the end of restore */;
> +	unsigned int			helpers_n;
> +	/* * * * * * * * * * * * * * * * * * * * */
> +
>  	unsigned long			premmapped_addr;
>  	unsigned long			premmapped_len;
>  	rt_sigaction_t			sigchld_act;
> @@ -122,12 +146,6 @@ struct task_restore_args {
>  
>  	struct itimerval		itimers[3];
>  
> -	int				timer_n;
> -	struct restore_posix_timer	*posix_timers;
> -
> -	int				timerfd_n;
> -	struct restore_timerfd		*timerfd;
> -
>  	CredsEntry			creds;
>  	u32				cap_inh[CR_CAP_SIZE];
>  	u32				cap_prm[CR_CAP_SIZE];
> @@ -140,20 +158,8 @@ struct task_restore_args {
>  	u32				mm_saved_auxv_size;
>  	char				comm[TASK_COMM_LEN];
>  
> -	int				nr_rlim;
> -	struct rlimit			*rlims;
> -
> -	struct rst_tcp_sock		*tcp_socks;
> -	int				tcp_socks_nr;
> -
> -	struct rst_aio_ring		*rings;
> -	int				nr_rings;
> -
>  	int				fd_last_pid; /* sys.ns_last_pid for threads rst */
>  
> -	pid_t				*helpers /* the TASK_HELPERS to wait on at the end of restore */;
> -	int				n_helpers;
> -
>  	int				proc_attr_current;
>  	char				*lsm_profile;
>  	int				lsm_profile_len;
> diff --git a/pie/restorer.c b/pie/restorer.c
> index 8713c6a..893928b 100644
> --- a/pie/restorer.c
> +++ b/pie/restorer.c
> @@ -273,7 +273,7 @@ static void restore_rlims(struct task_restore_args *ta)
>  {
>  	int r;
>  
> -	for (r = 0; r < ta->nr_rlim; r++) {
> +	for (r = 0; r < ta->rlims_n; r++) {
>  		struct krlimit krlim;
>  
>  		krlim.rlim_cur = ta->rlims[r].rlim_cur;
> @@ -378,7 +378,7 @@ long __export_restore_thread(struct thread_restore_args *args)
>  
>  	restore_finish_stage(CR_STATE_RESTORE);
>  
> -	if (restore_signals(args->siginfo, args->siginfo_nr, false))
> +	if (restore_signals(args->siginfo, args->siginfo_n, false))
>  		goto core_restore_end;
>  
>  	restore_finish_stage(CR_STATE_RESTORE_SIGCHLD);
> @@ -469,7 +469,7 @@ static void rst_tcp_socks_all(struct task_restore_args *ta)
>  {
>  	int i;
>  
> -	for (i = 0; i < ta->tcp_socks_nr; i++)
> +	for (i = 0; i < ta->tcp_socks_n; i++)
>  		rst_tcp_repair_off(&ta->tcp_socks[i]);
>  }
>  
> @@ -599,7 +599,7 @@ static int create_posix_timers(struct task_restore_args *args)
>  	timer_t next_id;
>  	struct sigevent sev;
>  
> -	for (i = 0; i < args->timer_n; i++) {
> +	for (i = 0; i < args->posix_timers_n; i++) {
>  		sev.sigev_notify = args->posix_timers[i].spt.it_sigev_notify;
>  		sev.sigev_signo = args->posix_timers[i].spt.si_signo;
>  		sev.sigev_value.sival_ptr = args->posix_timers[i].spt.sival_ptr;
> @@ -635,7 +635,7 @@ static void restore_posix_timers(struct task_restore_args *args)
>  	int i;
>  	struct restore_posix_timer *rt;
>  
> -	for (i = 0; i < args->timer_n; i++) {
> +	for (i = 0; i < args->posix_timers_n; i++) {
>  		rt = &args->posix_timers[i];
>  		sys_timer_settime((timer_t)rt->spt.it_id, 0, &rt->val, NULL);
>  	}
> @@ -721,7 +721,7 @@ static int wait_helpers(struct task_restore_args *task_args)
>  {
>  	int i;
>  
> -	for (i = 0; i < task_args->n_helpers; i++) {
> +	for (i = 0; i < task_args->helpers_n; i++) {
>  		int status;
>  		pid_t pid = task_args->helpers[i];
>  
> @@ -789,7 +789,7 @@ long __export_restore_task(struct task_restore_args *args)
>  
>  	task_entries = args->task_entries;
>  	helpers = args->helpers;
> -	n_helpers = args->n_helpers;
> +	n_helpers = args->helpers_n;
>  	*args->breakpoint = rst_sigreturn;
>  
>  	ksigfillset(&act.rt_sa_mask);
> @@ -815,8 +815,8 @@ long __export_restore_task(struct task_restore_args *args)
>  		goto core_restore_end;
>  
>  	/* Shift private vma-s to the left */
> -	for (i = 0; i < args->nr_vmas; i++) {
> -		vma_entry = args->tgt_vmas + i;
> +	for (i = 0; i < args->vmas_n; i++) {
> +		vma_entry = args->vmas + i;
>  
>  		if (!vma_entry_is_private(vma_entry))
>  			continue;
> @@ -833,8 +833,8 @@ long __export_restore_task(struct task_restore_args *args)
>  	}
>  
>  	/* Shift private vma-s to the right */
> -	for (i = args->nr_vmas - 1; i >= 0; i--) {
> -		vma_entry = args->tgt_vmas + i;
> +	for (i = args->vmas_n - 1; i >= 0; i--) {
> +		vma_entry = args->vmas + i;
>  
>  		if (!vma_entry_is_private(vma_entry))
>  			continue;
> @@ -853,8 +853,8 @@ long __export_restore_task(struct task_restore_args *args)
>  	/*
>  	 * OK, lets try to map new one.
>  	 */
> -	for (i = 0; i < args->nr_vmas; i++) {
> -		vma_entry = args->tgt_vmas + i;
> +	for (i = 0; i < args->vmas_n; i++) {
> +		vma_entry = args->vmas + i;
>  
>  		if (!vma_entry_is(vma_entry, VMA_AREA_REGULAR))
>  			continue;
> @@ -874,12 +874,12 @@ long __export_restore_task(struct task_restore_args *args)
>  	/*
>  	 * Proxify vDSO.
>  	 */
> -	for (i = 0; i < args->nr_vmas; i++) {
> -		if (vma_entry_is(&args->tgt_vmas[i], VMA_AREA_VDSO) ||
> -		    vma_entry_is(&args->tgt_vmas[i], VMA_AREA_VVAR)) {
> +	for (i = 0; i < args->vmas_n; i++) {
> +		if (vma_entry_is(&args->vmas[i], VMA_AREA_VDSO) ||
> +		    vma_entry_is(&args->vmas[i], VMA_AREA_VVAR)) {
>  			if (vdso_proxify("dumpee", &args->vdso_sym_rt,
>  					 args->vdso_rt_parked_at,
> -					 i, args->tgt_vmas, args->nr_vmas))
> +					 i, args->vmas, args->vmas_n))
>  				goto core_restore_end;
>  			break;
>  		}
> @@ -890,8 +890,8 @@ long __export_restore_task(struct task_restore_args *args)
>  	 * Walk though all VMAs again to drop PROT_WRITE
>  	 * if it was not there.
>  	 */
> -	for (i = 0; i < args->nr_vmas; i++) {
> -		vma_entry = args->tgt_vmas + i;
> +	for (i = 0; i < args->vmas_n; i++) {
> +		vma_entry = args->vmas + i;
>  
>  		if (!(vma_entry_is(vma_entry, VMA_AREA_REGULAR)))
>  			continue;
> @@ -907,10 +907,10 @@ long __export_restore_task(struct task_restore_args *args)
>  	/*
>  	 * Finally restore madivse() bits
>  	 */
> -	for (i = 0; i < args->nr_vmas; i++) {
> +	for (i = 0; i < args->vmas_n; i++) {
>  		unsigned long m;
>  
> -		vma_entry = args->tgt_vmas + i;
> +		vma_entry = args->vmas + i;
>  		if (!vma_entry->has_madv || !vma_entry->madv)
>  			continue;
>  
> @@ -936,7 +936,7 @@ long __export_restore_task(struct task_restore_args *args)
>  	 * up AIO rings.
>  	 */
>  
> -	for (i = 0; i < args->nr_rings; i++) {
> +	for (i = 0; i < args->rings_n; i++) {
>  		struct rst_aio_ring *raio = &args->rings[i];
>  		unsigned long ctx = 0;
>  		int ret;
> @@ -1157,11 +1157,11 @@ long __export_restore_task(struct task_restore_args *args)
>  
>  	sys_sigaction(SIGCHLD, &args->sigchld_act, NULL, sizeof(k_rtsigset_t));
>  
> -	ret = restore_signals(args->siginfo, args->siginfo_nr, true);
> +	ret = restore_signals(args->siginfo, args->siginfo_n, true);
>  	if (ret)
>  		goto core_restore_end;
>  
> -	ret = restore_signals(args->t->siginfo, args->t->siginfo_nr, false);
> +	ret = restore_signals(args->t->siginfo, args->t->siginfo_n, false);
>  	if (ret)
>  		goto core_restore_end;
>  
> -- 
> 1.9.3
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu


More information about the CRIU mailing list