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

[Pavel Emelyanov]

On 04/13/2012 12:52 PM, Cyrill Gorcunov wrote:
> On Fri, Apr 13, 2012 at 12:49:28PM +0400, Cyrill Gorcunov wrote:
>>>> + * Note, this imples the caller code will restore the original
>>>
>>> implies
>>>
>>
>> Thanks Kir. I'll update and send it out.
>>
> 
> Pavel, pick this one instead.
> 
> 	Cyrill

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

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

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

> +	if (vma_entry->start == (long)args->mm.mm_start_data ||
> +		vma_entry->end == (long)args->mm.mm_end_data) {
> +		flags |= PROT_READ | PROT_WRITE;
> +		flags &= ~PROT_EXEC;
> +
> +		if (vma_entry->start == (long)args->mm.mm_start_data)
> +			ret |= sys_prctl_safe(PR_SET_MM, PR_SET_MM_START_DATA,
> +					      (long)args->mm.mm_start_data, 0);
> +		else
> +			ret |= sys_prctl_safe(PR_SET_MM, PR_SET_MM_END_DATA,
> +					      (long)args->mm.mm_end_data, 0);
> +	}

Huh? What do you calculate flags here for?