+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
