Re: Kernel page fault in vmalloc_fault() after a preempted ioremap

From: Kani, Toshi
Date: Thu Mar 08 2018 - 16:43:37 EST


On Thu, 2018-03-08 at 14:34 -0600, Gratian Crisan wrote:
> Hi all,
>
> We are seeing kernel page faults happening on module loads with certain
> drivers like the i915 video driver[1]. This was initially discovered on
> a 4.9 PREEMPT_RT kernel. It takes 5 days on average to reproduce using a
> simple reboot loop test. Looking at the code paths involved I believe
> the issue is still present in the latest vanilla kernel.
>
> Some relevant points are:
>
> * x86_64 CPU: Intel Atom E3940
>
> * CONFIG_HUGETLBFS is not set (which also gates CONFIG_HUGETLB_PAGE)
>
> Based on function traces I was able to gather the sequence of events is:
>
> 1. Driver starts a ioremap operation for a region that is PMD_SIZE in
> size (or PUD_SIZE).
>
> 2. The ioremap() operation is preempted while it's in the middle of
> setting up the page mappings:
> ioremap_page_range->...->ioremap_pmd_range->pmd_set_huge <<preempted>>
>
> 3. Unrelated tasks run. Traces also include some cross core scheduling
> IPI calls.
>
> 4. Driver resumes execution finishes the ioremap operation and tries to
> access the newly mapped IO region. This triggers a vmalloc fault.
>
> 5. The vmalloc_fault() function hits a kernel page fault when trying to
> dereference a non-existent *pte_ref.
>
> The reason this happens is the code paths called from ioremap_page_range()
> make different assumptions about when a large page (pud/pmd) mapping can be
> used versus the code paths in vmalloc_fault().
>
> Using the PMD sized ioremap case as an example (the PUD case is similar):
> ioremap_pmd_range() calls ioremap_pmd_enabled() which is gated by
> CONFIG_HAVE_ARCH_HUGE_VMAP. On x86_64 this will return true unless the
> "nohugeiomap" kernel boot parameter is passed in.
>
> On the other hand, in the rare case when a page fault happens in the
> ioremap'ed region, vmalloc_fault() calls the pmd_huge() function to check
> if a PMD page is marked huge or if it should go on and get a reference to
> the PTE. However pmd_huge() is conditionally compiled based on the user
> configured CONFIG_HUGETLB_PAGE selected by CONFIG_HUGETLBFS. If the
> CONFIG_HUGETLBFS option is not enabled pmd_huge() is always defined to be
> 0.
>
> The end result is an OOPS in vmalloc_fault() when the non-existent pte_ref
> is dereferenced because the test for pmd_huge() failed.
>
> Commit f4eafd8bcd52 ("x86/mm: Fix vmalloc_fault() to handle large pages
> properly") attempted to fix the mismatch between ioremap() and
> vmalloc_fault() with regards to huge page handling but it missed this use
> case.
>
> I am working on a simpler reproducing case however so far I've been
> unsuccessful in re-creating the conditions that trigger the vmalloc fault
> in the first place. Adding explicit scheduling points in
> ioremap_pmd_range/pmd_set_huge doesn't seem to be sufficient. Ideas
> appreciated.
>
> Any thoughts on what a correct fix would look like? Should the ioremap
> code paths respect the HUGETLBFS config or would it be better for the
> vmalloc fault code paths to match the tests used in ioremap and not rely
> on the HUGETLBFS option being enabled?

Thanks for the report and analysis! I believe pud_large() and
pmd_large() should have been used here. I will try to reproduce the
issue and verify the fix.

-Toshi