: [CRIU] [PATCH] restorer: Make sure the protection on code/data mm areas do fit the kernel requirements

Cyrill Gorcunov gorcunov at openvz.org
Fri Apr 13 05:19:53 EDT 2012


On Fri, Apr 13, 2012 at 01:06:20PM +0400, Pavel Emelyanov wrote:
> > +static u64 restore_mapping(struct task_restore_core_args *args,
> > +			   const struct vma_entry *vma_entry)
> 
> What args is here for? It's not required in this fn.
> 

yeah, leftover. I though initially to move this helper into restore_mapping,
will drop it, thanks.

> > -		if (vma_entry->prot & PROT_WRITE)
> > -			continue;
> > -
> > +		if (setup_mm_special(args, vma_entry)) {
> > +			write_num_n(__LINE__);
> > +			write_num_n(ret);
> > +			goto core_restore_end;
> > +		}
> 
> This looks wrong. Non code/data mappings will be mprotected, while
> shouldn't

Nope, the setup_mm_special will test for mm::fields and vma->start/end,
and will skip vmas which lay outside of precise code/data start/end
addresses. And note that it's called as

	setup_mm_special
	sys_mprotect

thus the former protection restored then.

> 
> > +	if (vma_entry->start == (long)args->mm.mm_start_code ||
> > +	    vma_entry->end == (long)args->mm.mm_end_code) {
> > +		flags |= PROT_EXEC | PROT_READ;
> > +		flags &= ~PROT_WRITE;
> 
> This check doesn't look correct. We should check for intersection, not
> for exact match, shouldn't we?

Strictly speaking, yes. moreover it seems we need more tricky approach
here in case of some vmas were dropped at checkpoint time.

Pavel, drop this patch completely, I'll rework it.

> 
> Huh? What do you calculate flags here for?
> 

the kernel internally looks for corresponding VMA and test the flags
such VMA has, thus say .text VMAs must not be writable when being
tuned up via prctl call.

	Cyrill


More information about the CRIU mailing list