On 10 April 2017 at 10:41, Mark Rutland <mark.rutland@xxxxxxx> wrote: > On Fri, Apr 07, 2017 at 04:51:16PM +0100, Ard Biesheuvel wrote: >> On 7 April 2017 at 16:47, James Morse <james.morse@xxxxxxx> wrote: >> > On 24/03/17 13:24, Ard Biesheuvel wrote: >> >> Update the allocation logic for the virtual mapping of the UEFI runtime >> >> services to start from a randomized base address if KASLR is in effect, >> >> and if the UEFI firmware exposes an implementation of EFI_RNG_PROTOCOL. >> >> >> >> This makes it more difficult to predict the location of exploitable >> >> data structures in the runtime UEFI firmware, which increases robustness >> >> against attacks. Note that these regions are only mapped during the >> >> time a runtime service call is in progress, and only on a single CPU >> >> at a time, bit give the lack of a downside, let's enable it nonetheless. >> > >> > With next-20170407 on Seattle Overdrive, I get an SError[0] on boot: > >> > * Revert "ef/libstub/arm/arm64: Randomize the base of the UEFI rt services region" >> > >> > At which point the machine start booting to a prompt again, (its noisier than >> > usual but looks like double-printing). > >> That is quite interesting, to be honest, because that patch should >> effectively be a NOP on systems that do not implement >> EFI_RNG_PROTOCOL. > > FWIW, I'm also seeing a crash as a result of this patch, on a Juno R1 > with an EFI_RNG_PROTOCOL implementation, with next-20170410 defconfig. > I'm using the Linaro 15.08 toolchain. > > ---- > EFI stub: Booting Linux Kernel... > EFI stub: Using DTB from configuration table > > > WARNING: this system is using an unsafe pseudo-random implementation of > RngLib! > > > > Synchronous Exception at 0x00000000F80FC7E8 > ---- > > If I replace the !nokaslr() in efi_entry() with false, my kernel boots. > > If I replace !nokaslr() with true, it fails to boot, even with the call > to efi_get_random_bytes() replaced with status = EFI_ERROR. > > The problem appears to be the use of TASK_SIZE, since on arm64 that's: > > #define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \ > TASK_SIZE_32 : TASK_SIZE_64) > > ... which would mean that the stub would be trying to dereference whatever it > found in sp_el0. Looking at objdump: > > 0000000000000160 <efi_entry>: > ... > 4b4: 94000000 bl 0 <nokaslr> > 4b8: 35000280 cbnz w0, 508 <efi_entry+0x3a8> > 4bc: d5384100 mrs x0, sp_el0 > 4c0: f9400004 ldr x4, [x0] > ... > > So I think we should revert this for now. > Thanks for debugging this! I will get a fixed version out asap -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
