Re: A mapcount riddle

[Mike Kravetz <mike.kravetz@oracle.com>]

On 01/24/23 18:00, Peter Xu wrote:
> On Tue, Jan 24, 2023 at 12:56:24PM -0800, Mike Kravetz wrote:
> > At first thought this seems bad.  However, I believe this has been the
> > behavior since hugetlb PMD sharing was introduced in 2006 and I am
> > unaware of any reported issues.  I did a audit of code looking at
> > mapcount.  In addition to the above issue with smaps, there appears
> > to be an issue with 'migrate_pages' where shared pages could be migrated
> > without appropriate privilege.
> > 
> > 	/* With MPOL_MF_MOVE, we migrate only unshared hugepage. */
> > 	if (flags & (MPOL_MF_MOVE_ALL) ||
> > 	    (flags & MPOL_MF_MOVE && page_mapcount(page) == 1)) {
> > 		if (isolate_hugetlb(page, qp->pagelist) &&
> > 			(flags & MPOL_MF_STRICT))
> > 			/*
> > 			 * Failed to isolate page but allow migrating pages
> > 			 * which have been queued.
> > 			 */
> > 			ret = 1;
> > 	}
> > 
> > I will prepare fixes for both of these.  However, I wanted to ask if
> > anyone has ideas about other potential issues with this?
> 
> This reminded me whether things should be checked already before this
> happens.  E.g. when trying to share pmd, whether it makes sense to check
> vma mempolicy before doing so?

Not sure I understand your question.  Are you questioning whether we should
enter into pmd sharing if mempolicy allows movement to another node?
Wouldn't this be the 'normal' case on a multi-node system?

> Then the question is if pmd sharing only happens with the vma that shares
> the same memory policy, whether above mapcount==1 check would be acceptable
> even if it's shared by multiple processes.

I am not a mempolicy expert, but that would still involve moving pages
mapped by another process.  For that CAP_SYS_NICE is required.  So, my
opinion would be that it is not allowed even if mempolicy is the same.

> Besides, I'm also curious on the planned fix too regarding the two issues
> mentioned.

My planned 'fix' is to simply check for shared a PMD
(page_count(virt_to_page(pte))) to determine if page with mapcount == 1
is shared.
-- 
Mike Kravetz