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

Pavel Emelyanov xemul at parallels.com
Fri Apr 13 05:24:40 EDT 2012


>>> -		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.

Before patch RO vmas that have nothing to do with code/data will
not be mprotect-ed. After patch -- will. The "continue" is removed!

>>
>>> +	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.

You don't use the flags value after this AT ALL, wtf???

> 	Cyrill
> .
> 



More information about the CRIU mailing list