Re: [PATCH] hugetlb: unshare some PMDs when splitting VMAs

[James Houghton]

> Thanks James.  I am just trying to determine if we may have any issues/bugs/
> undesired behavior based on this today.  Consider the cases mentioned above:
> mbind - I do not think this would cause any user visible issues.  mbind is
>         only dealing with newly allocated pages.  We do not unshare as the
>         result of a mbind call today.
> madvise(MADV_DONTDUMP) - It looks like this results in a flag (VM_DONTDUMP)
>         being set on the vma.  So, I do not believe sharing page tables
>         would cause any user visible issue.
>
> One somewhat strange things about two vmas after split sharing a PMD is
> that operations on one VMA can impact the other.  For example, suppose
> A VMA split via mbind happens.  Then later, mprotect is done on one of
> the VMAs in the range that is shared.  That would result in the area being
> unshared in both VMAs.  So, the 'other' vma could see minor faults after
> the mprotect.
>
> Just curious if you (or anyone) knows of a user visible issue caused by this
> today.  Trying to determine if we need a Fixes: tag.

I think I've come up with one... :) It only took many many hours of
staring at code to come up with:

1. Fault in PUD_SIZE-aligned hugetlb mapping
2. fork() (to actually share the PMDs)
3. Split VMA with MADV_DONTDUMP
4. Register the lower piece of the newly split VMA with
UFFDIO_REGISTER_MODE_WRITEPROTECT (this will call
hugetlb_unshare_all_pmds, but it will not attempt to unshare in the
unaligned bits now)
5. Now calling UFFDIO_WRITEPROTECT will drop into
hugetlb_change_protection and succeed in unsharing. That will hit the
WARN_ON_ONCE and *not write-protect anything*.

I'll see if I can confirm that this is indeed possible and send a
repro if it is.

60dfaad65a ("mm/hugetlb: allow uffd wr-protect none ptes") is the
commit that introduced the WARN_ON_ONCE; perhaps it's a good choice
for a Fixes: tag (if above is indeed true).

>
> Code changes look fine to me.

Thanks Mike!
- James