Re: [PATCH] hugetlbfs: fix kernel BUG at fs/hugetlbfs/inode.c:444!

From: Naoya Horiguchi
Date: Mon Nov 05 2018 - 20:34:52 EST


On Mon, Nov 05, 2018 at 01:23:15PM -0800, Mike Kravetz wrote:
> This bug has been experienced several times by Oracle DB team.
> The BUG is in the routine remove_inode_hugepages() as follows:
> /*
> * If page is mapped, it was faulted in after being
> * unmapped in caller. Unmap (again) now after taking
> * the fault mutex. The mutex will prevent faults
> * until we finish removing the page.
> *
> * This race can only happen in the hole punch case.
> * Getting here in a truncate operation is a bug.
> */
> if (unlikely(page_mapped(page))) {
> BUG_ON(truncate_op);
>
> In this case, the elevated map count is not the result of a race.
> Rather it was incorrectly incremented as the result of a bug in the
> huge pmd sharing code. Consider the following:
> - Process A maps a hugetlbfs file of sufficient size and alignment
> (PUD_SIZE) that a pmd page could be shared.
> - Process B maps the same hugetlbfs file with the same size and alignment
> such that a pmd page is shared.
> - Process B then calls mprotect() to change protections for the mapping
> with the shared pmd. As a result, the pmd is 'unshared'.
> - Process B then calls mprotect() again to chage protections for the
> mapping back to their original value. pmd remains unshared.
> - Process B then forks and process C is created. During the fork process,
> we do dup_mm -> dup_mmap -> copy_page_range to copy page tables. Copying
> page tables for hugetlb mappings is done in the routine
> copy_hugetlb_page_range.
>
> In copy_hugetlb_page_range(), the destination pte is obtained by:
> dst_pte = huge_pte_alloc(dst, addr, sz);
> If pmd sharing is possible, the returned pointer will be to a pte in
> an existing page table. In the situation above, process C could share
> with either process A or process B. Since process A is first in the
> list, the returned pte is a pointer to a pte in process A's page table.
>
> However, the following check for pmd sharing is in copy_hugetlb_page_range.
> /* If the pagetables are shared don't copy or take references */
> if (dst_pte == src_pte)
> continue;
>
> Since process C is sharing with process A instead of process B, the above
> test fails. The code in copy_hugetlb_page_range which follows assumes
> dst_pte points to a huge_pte_none pte. It copies the pte entry from
> src_pte to dst_pte and increments this map count of the associated page.
> This is how we end up with an elevated map count.
>
> To solve, check the dst_pte entry for huge_pte_none. If !none, this
> implies PMD sharing so do not copy.
>
> Signed-off-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx>

Reviewed-by: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>