Page fault while link_path_walk for path_len > 4060 bytes

Will Deacon will.deacon at arm.com
Mon Nov 6 17:19:03 PST 2017


On Sat, Nov 04, 2017 at 12:17:57AM +0000, Al Viro wrote:
> [looking through the old mail]
> 
> On Tue, Sep 12, 2017 at 09:26:16PM +0100, Will Deacon wrote:
> > > -    { do_page_fault,    SIGSEGV, SEGV_MAPERR,    "level 3 translation
> > > fault"    },
> > > +    { do_translation_fault,    SIGSEGV, SEGV_MAPERR,    "level 3
> > > translation fault"    },
> > > we will try with above changes and get back to you.
> > 
> > Did you test with this change?
> 
> FWIW, while that went in as commit 760bfb47c36a ("arm64: fault: Route pte
> translation faults via do_translation_fault"), I wonder if the same issue
> exists on arm.  It looks like the pagefault handler there is fairly
> similar to arm64 one and the same shortcut is present there.

So we never actually nailed this one down on arm64 (the problem mysteriously
disappeared iirc, and I failed to reproduce it on my systems) but I fixed it
anyway because it looked at least theoretically possible. In that case, yes,
I think ARM needs the same sort of fixes for both 2 and 3 level tables where
the handler branch directly to do_page_fault.

> The more I'm looking at that, the more it looks like we *really* need
> a comment in all instances of load_unaligned_zeropad() warning about
> that pitfall.  Something like
> /*
>  * Load an unaligned word from kernel space.
>  *
>  * In the (very unlikely) case of the word being a page-crosser
>  * and the next page not being mapped, take the exception and
>  * return zeroes in the non-existing part.
>  *
>  * NOTE: this relies upon the pagefault handler *NOT* blocking
>  * in such situation (fault in kernel mode on kernel address with
>  * exception fixup present).  Verify that for your architecture
>  * before using an equivalent of this approach.  Note that
>  * you can't count upon faulthandler_disabled() saving you;
>  * this function can be called e.g. under a spinlock on non-preempt
>  * kernels without pagefault_disable() done by caller.
>  */
> perhaps.  That property holds on x86, ppc and (now) arm64, but as
> arm64 case shows, it might not be true for other architectures.
> As the matter of fact, e.g. sparc64 (which will not use that
> thing for obvious reasons anyway) it is *not* true, etc.

I wonder if there's any mileage in a test module for this? That code rarely
(if ever) gets run in practice.

Will



More information about the linux-arm-kernel mailing list