Re: [PATCH v3] mm/ksm: fix ksm exec support for prctl

[David Hildenbrand <david@redhat.com>]

>>>    +static inline int ksm_execve(struct mm_struct *mm)
>>> +{
>>> +    if (test_bit(MMF_VM_MERGE_ANY, &mm->flags))
>>> +        return __ksm_enter(mm);
>>
>> As soon as we did the __ksm_enter(), we have to set MMF_VM_MERGEABLE.
>> I don't think it would be set right now, because:
>>
>> mm_alloc()->mm_init() will initialize the flags using
>>
>>      mm->flags = mmf_init_flags(current->mm->flags);
>>
>> Whereby MMF_INIT_MASK contains only MMF_VM_MERGE_ANY_MASK.
>>
>>
>> So we likely need a set_bit(MMF_VM_MERGEABLE, &mm->flags) here as
>> well. Otherwise ksm_exit() wouldn't clean up, and we might call
>> __ksm_enter() twice.
> 
> __ksm_enter() will set MMF_VM_MERGEABLE when it succeeds.
> 

Ah, indeed!

>>
>>
>> And now I wonder, when would be the right point to call __ksm_enter().
>>
>> __mmput() calls ksm_exit(). But for example, if __bprm_mm_init() fails
>> after __ksm_enter(), we will only call mmdrop(), not cleaning up ...
>> so that looks wrong.
> Yes, I forgot cleanup in error path. ksm_exit() should be called when
> __bprm_mm_init() fails after __ksm_enter().
>>
>> We would have to make sure we call __ksm_enter() only once we know
>> that callers will call mm_put(). that is the case once we return from
>> bprm_mm_init() with success.
>>
>> Maybe move the ksm_execve() to bprm_mm_init(), adding a comment that
>> cleanup will only happen during mm_put(), so it's harder to mess up in
>> the future?
>>
> __ksm_enter() should be called under mmap write lock to avoid race with
> ksmd. So we can't move ksm_execve() to bprm_mm_init().

Makes sense. We have to be careful that this won't silently break in the 
future. Better add some comment as well.

-- 
Cheers,

David / dhildenb