: [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