[linux-next PATCH] arm64: fix kernel crash with 48-bit VA and 64KB granule

[Ard Biesheuvel]

On 6 January 2016 at 09:52, Dennis Chen <dennis.chen at arm.com> wrote:
> On Wed, Jan 06, 2016 at 08:42:20AM +0100, Ard Biesheuvel wrote:
>> On 6 January 2016 at 08:38, Dennis Chen <dennis.chen at arm.com> wrote:
>> > On Wed, Jan 06, 2016 at 08:13:24AM +0100, Ard Biesheuvel wrote:
>> >> On 6 January 2016 at 07:14, Dennis Chen <dennis.chen at arm.com> wrote:
>> >> > On Tue, Jan 05, 2016 at 09:56:03AM +0000, Catalin Marinas wrote:
>> >> >> On Tue, Jan 05, 2016 at 04:40:44PM +0800, Dennis Chen wrote:
>> >> >> > On Tue, Jan 05, 2016 at 09:38:11AM +0100, Ard Biesheuvel wrote:
>> >> >> > > >> On 5 January 2016 at 03:18, Dennis Chen <dennis.chen at arm.com> wrote:
>> >> >> > > >> > The commit 3400749b5a22 ("arm64/efi: refactor EFI init and runtime
>> >> >> > > >> > code for reuse by 32-bit ARM") uses pgd_alloc() to allocate space for
>> >> >> > > >> > efi_mm.pgd while not the static efi_pgd[], since this function will be
>> >> >> > > >> > called with early_initcall, which results in the pgd_cache used by
>> >> >> > > >> > pgd_alloc() has not been initialized yet, kernel will hang in this
>> >> >> > > >> > case. This patch is trying to make the pgd_cache_init() called before
>> >> >> > > >> > arm_enable_runtime_services() by changing its core_initcall to
>> >> >> > > >> > early_initcall.
>> >> >> > > >> >
>> >> >> > > >> > Signed-off-by: Dennis Chen <dennis.chen at arm.com>
>> >> >> > > >> > Tested-by: Sudeep Holla <sudeep.holla at arm.com>
>> >> >> > > >> >
>> >> >> > > >> > Cc: Will Deacon <will.deacon at arm.com>
>> >> >> > > >> > Cc: Catalin Marinas <catalin.marinas at arm.com>
>> >> >> > > >> > Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
>> >> >> > > >> > Cc: Sudeep Holla <sudeep.holla at arm.com>
>> >> >> > > >> > ---
>> >> >> > > >> >  arch/arm64/mm/pgd.c | 2 +-
>> >> >> > > >> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> >> > > >> >
>> >> >> > > >> > diff --git a/arch/arm64/mm/pgd.c b/arch/arm64/mm/pgd.c
>> >> >> > > >> > index cb3ba1b..859a788 100644
>> >> >> > > >> > --- a/arch/arm64/mm/pgd.c
>> >> >> > > >> > +++ b/arch/arm64/mm/pgd.c
>> >> >> > > >> > @@ -56,4 +56,4 @@ static int __init pgd_cache_init(void)
>> >> >> > > >> >                                               SLAB_PANIC, NULL);
>> >> >> > > >> >         return 0;
>> >> >> > > >> >  }
>> >> >> > > >> > -core_initcall(pgd_cache_init);
>> >> >> > > >> > +early_initcall(pgd_cache_init);
>> >> >> [...]
>> >> >> > > Well, since arm_enable_runtime_services() is an early_initcall()
>> >> >> > > itself, how are you guaranteeing the ordering between the two? Link
>> >> >> > > order?
>> >> >> >
>> >> >> > Link order.
>> >> >>
>> >> >> And can you explain how this works, what guarantees it gives?
>> >> >>
>> >> > You can take a look at include/asm-generic/vmlinux.ldx.h: INIT_CALLS macro,
>> >> > for the init call section, early_initcall is the first chuck in the section,
>> >> > followed by LEVEL[0-7]. For the same level, the layout order is determined
>> >> > by the link order, ARCH is always precedence over the drivers. Catalin, are
>> >> > you giving me a kernel examination? :)
>> >> >
>> >>
>> >> We all know how initcalls are implemented. The question is how you are
>> >> going to ensure that the early_initcall() that initializes the PGD
>> >> cache is always invoked before the early_initcall() that creates the
>> >> UEFI runtime page tables.
>> >>
>> >> And saying that the currently observed link order happens to be
>> >> correct is not good enough. We need to be sure that, even if we change
>> >> the link order, or switch to LTO at some point, things don't suddenly
>> >> break again.
>> >>
>> > Well, if the build system changes the link order, it can't make sure it will break something
>> > unexpectedly, needless to say the pgd_cache_init here at all.
>>
>> That is no excuse to introduce yet another failure mode.
>>
>> > Do you think the kernel
>> > will change its currently link order policy? If the answer is yes, what's the benefit?
>> >
>>
>> It does not matter what I think. You seem to think that the link order
>> is set in stone, so it is you who should argue why that is a
>> reasonable assumption.
>>
> OK, If you think the link order is volatile, how can you guarantee the arm_enable_runtime_services with early_initcall
> always work in a volatile link order environment?
>

By not relying on other early_initcalls