| Message ID | 161920fc31ec4168290ca31b3e4ac7a75ac1df6b.camel@kernel.crashing.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] arm64: efi: kaslr: Fix occasional random alloc (and boot) failure | expand |
On Tue, 20 Jul 2021 at 13:14, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > If efi_random_alloc() fails, we still try to use EFI_KIMG_ALIGN > instead of MIN_KIMG_ALIGN to check the kernel image alignment, > which is incorrect, we need to fallback to MIN_KIMG_ALIGN (2M). > Why? Relocatable kernels can happily execute from any 64k aligned address, and the PE/COFF header carries this value of 64k as the minimum alignment. > This removes the not-that-useful min_kimg_align helper and instead > uses the appropriate aligment in the respective call sites: > > efi_random_alloc() always wants EFI_KIMG_ALIGN as this is only > used when kaslr is on, and all other cases go into alignment > check code which always need to check (and enforce) MIN_KIMG_ALIGN > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Fixes: 7c116db24d94 (efi/libstub/arm64: Retain 2MB kernel Image alignment if !KASLR) > --- > drivers/firmware/efi/libstub/arm64-stub.c | 27 ++++++++++------------- > 1 file changed, 12 insertions(+), 15 deletions(-) > > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c > index 7bf0a7acae5e..e264ff90ba03 100644 > --- a/drivers/firmware/efi/libstub/arm64-stub.c > +++ b/drivers/firmware/efi/libstub/arm64-stub.c > @@ -34,18 +34,6 @@ efi_status_t check_platform_features(void) > return EFI_SUCCESS; > } > > -/* > - * Although relocatable kernels can fix up the misalignment with respect to > - * MIN_KIMG_ALIGN, the resulting virtual text addresses are subtly out of > - * sync with those recorded in the vmlinux when kaslr is disabled but the > - * image required relocation anyway. Therefore retain 2M alignment unless > - * KASLR is in use. > - */ > -static u64 min_kimg_align(void) > -{ > - return efi_nokaslr ? MIN_KIMG_ALIGN : EFI_KIMG_ALIGN; > -} > - > efi_status_t handle_kernel_image(unsigned long *image_addr, > unsigned long *image_size, > unsigned long *reserve_addr, > @@ -84,15 +72,24 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, > /* > * If KASLR is enabled, and we have some randomness available, > * locate the kernel at a randomized offset in physical memory. > + * > + * In that case, we don't need to preserve the 2M alignment > */ > - status = efi_random_alloc(*reserve_size, min_kimg_align(), > + status = efi_random_alloc(*reserve_size, EFI_KIMG_ALIGN, > reserve_addr, phys_seed); > } else { > status = EFI_OUT_OF_RESOURCES; > } > > if (status != EFI_SUCCESS) { > - if (IS_ALIGNED((u64)_text, min_kimg_align())) { > + /* > + * Although relocatable kernels can fix up the misalignment with respect to > + * MIN_KIMG_ALIGN, the resulting virtual text addresses are subtly out of > + * sync with those recorded in the vmlinux when kaslr is disabled but the > + * image required relocation anyway. Therefore retain 2M alignment unless > + * KASLR is in use. > + */ > + if (IS_ALIGNED((u64)_text, MIN_KIMG_ALIGN)) { > /* > * Just execute from wherever we were loaded by the > * UEFI PE/COFF loader if the alignment is suitable. > @@ -103,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, > } > > status = efi_allocate_pages_aligned(*reserve_size, reserve_addr, > - ULONG_MAX, min_kimg_align()); > + ULONG_MAX, MIN_KIMG_ALIGN); > > if (status != EFI_SUCCESS) { > efi_err("Failed to relocate kernel\n"); > > >
On Tue, 2021-07-20 at 14:57 +0200, Ard Biesheuvel wrote: > On Tue, 20 Jul 2021 at 13:14, Benjamin Herrenschmidt > <benh@kernel.crashing.org> wrote: > > If efi_random_alloc() fails, we still try to use EFI_KIMG_ALIGN > > instead of MIN_KIMG_ALIGN to check the kernel image alignment, > > which is incorrect, we need to fallback to MIN_KIMG_ALIGN (2M). > > > > Why? Relocatable kernels can happily execute from any 64k aligned > address, and the PE/COFF header carries this value of 64k as the > minimum alignment. This is not what I'm changing. If you look at the code before the patch, it was *already* only allowing 64k alignment with kaslr enabled (commit 7c116db24d94). There's even a big fat comment explaining why though it could use more details. In any case, the code isn't even testing for CONFIG_RELOCATABLE today, it makes its decisions entirely based on kaslr and doesn't properly handle the case where efi_random_alloc() fails. This fixes it. Cheers, Ben. > > This removes the not-that-useful min_kimg_align helper and instead > > uses the appropriate aligment in the respective call sites: > > > > efi_random_alloc() always wants EFI_KIMG_ALIGN as this is only > > used when kaslr is on, and all other cases go into alignment > > check code which always need to check (and enforce) MIN_KIMG_ALIGN > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > Fixes: 7c116db24d94 (efi/libstub/arm64: Retain 2MB kernel Image alignment if !KASLR) > > --- > > drivers/firmware/efi/libstub/arm64-stub.c | 27 ++++++++++------------- > > 1 file changed, 12 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c > > index 7bf0a7acae5e..e264ff90ba03 100644 > > --- a/drivers/firmware/efi/libstub/arm64-stub.c > > +++ b/drivers/firmware/efi/libstub/arm64-stub.c > > @@ -34,18 +34,6 @@ efi_status_t check_platform_features(void) > > return EFI_SUCCESS; > > } > > > > -/* > > - * Although relocatable kernels can fix up the misalignment with respect to > > - * MIN_KIMG_ALIGN, the resulting virtual text addresses are subtly out of > > - * sync with those recorded in the vmlinux when kaslr is disabled but the > > - * image required relocation anyway. Therefore retain 2M alignment unless > > - * KASLR is in use. > > - */ > > -static u64 min_kimg_align(void) > > -{ > > - return efi_nokaslr ? MIN_KIMG_ALIGN : EFI_KIMG_ALIGN; > > -} > > - > > efi_status_t handle_kernel_image(unsigned long *image_addr, > > unsigned long *image_size, > > unsigned long *reserve_addr, > > @@ -84,15 +72,24 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, > > /* > > * If KASLR is enabled, and we have some randomness available, > > * locate the kernel at a randomized offset in physical memory. > > + * > > + * In that case, we don't need to preserve the 2M alignment > > */ > > - status = efi_random_alloc(*reserve_size, min_kimg_align(), > > + status = efi_random_alloc(*reserve_size, EFI_KIMG_ALIGN, > > reserve_addr, phys_seed); > > } else { > > status = EFI_OUT_OF_RESOURCES; > > } > > > > if (status != EFI_SUCCESS) { > > - if (IS_ALIGNED((u64)_text, min_kimg_align())) { > > + /* > > + * Although relocatable kernels can fix up the misalignment with respect to > > + * MIN_KIMG_ALIGN, the resulting virtual text addresses are subtly out of > > + * sync with those recorded in the vmlinux when kaslr is disabled but the > > + * image required relocation anyway. Therefore retain 2M alignment unless > > + * KASLR is in use. > > + */ > > + if (IS_ALIGNED((u64)_text, MIN_KIMG_ALIGN)) { > > /* > > * Just execute from wherever we were loaded by the > > * UEFI PE/COFF loader if the alignment is suitable. > > @@ -103,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, > > } > > > > status = efi_allocate_pages_aligned(*reserve_size, reserve_addr, > > - ULONG_MAX, min_kimg_align()); > > + ULONG_MAX, MIN_KIMG_ALIGN); > > > > if (status != EFI_SUCCESS) { > > efi_err("Failed to relocate kernel\n"); > > > > > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, 20 Jul 2021 at 15:10, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > On Tue, 2021-07-20 at 14:57 +0200, Ard Biesheuvel wrote: > > On Tue, 20 Jul 2021 at 13:14, Benjamin Herrenschmidt > > <benh@kernel.crashing.org> wrote: > > > If efi_random_alloc() fails, we still try to use EFI_KIMG_ALIGN > > > instead of MIN_KIMG_ALIGN to check the kernel image alignment, > > > which is incorrect, we need to fallback to MIN_KIMG_ALIGN (2M). > > > > > > > Why? Relocatable kernels can happily execute from any 64k aligned > > address, and the PE/COFF header carries this value of 64k as the > > minimum alignment. > > This is not what I'm changing. If you look at the code before the > patch, it was *already* only allowing 64k alignment with kaslr enabled > (commit 7c116db24d94). There's even a big fat comment explaining why > though it could use more details. > But that only takes effect if efi_nokaslr is true, which is the default if CONFIG_RANDOMIZE_BASE is not set > In any case, the code isn't even testing for CONFIG_RELOCATABLE today, No, it tests for CONFIG_RANDOMIZE_BASE, which implies CONFIG_RELOCATABLE. > it makes its decisions entirely based on kaslr and doesn't properly > handle the case where efi_random_alloc() fails. > > This fixes it. > You are replacing min_kimg_align() with MIN_KIMG_ALIGN in a place where it could return either value: efi_nokaslr will be false by default on relocatable kernels, in which case min_kimg_align() will return EFI_KIMG_ALIGN, unless you specifically request KASLR to be disabled. The result is that relocatable kernels that would not require to be moved will now be moved to a 2 MB aligned offset before booting them. Similarly for the efi_allocate_pages_aligned() call: that call would only request 64k alignment before on a relocatable kernel if booting without randomization. > > > This removes the not-that-useful min_kimg_align helper and instead > > > uses the appropriate aligment in the respective call sites: > > > > > > efi_random_alloc() always wants EFI_KIMG_ALIGN as this is only > > > used when kaslr is on, and all other cases go into alignment > > > check code which always need to check (and enforce) MIN_KIMG_ALIGN > > > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > > Fixes: 7c116db24d94 (efi/libstub/arm64: Retain 2MB kernel Image alignment if !KASLR) > > > --- > > > drivers/firmware/efi/libstub/arm64-stub.c | 27 ++++++++++------------- > > > 1 file changed, 12 insertions(+), 15 deletions(-) > > > > > > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c > > > index 7bf0a7acae5e..e264ff90ba03 100644 > > > --- a/drivers/firmware/efi/libstub/arm64-stub.c > > > +++ b/drivers/firmware/efi/libstub/arm64-stub.c > > > @@ -34,18 +34,6 @@ efi_status_t check_platform_features(void) > > > return EFI_SUCCESS; > > > } > > > > > > -/* > > > - * Although relocatable kernels can fix up the misalignment with respect to > > > - * MIN_KIMG_ALIGN, the resulting virtual text addresses are subtly out of > > > - * sync with those recorded in the vmlinux when kaslr is disabled but the > > > - * image required relocation anyway. Therefore retain 2M alignment unless > > > - * KASLR is in use. > > > - */ > > > -static u64 min_kimg_align(void) > > > -{ > > > - return efi_nokaslr ? MIN_KIMG_ALIGN : EFI_KIMG_ALIGN; > > > -} > > > - > > > efi_status_t handle_kernel_image(unsigned long *image_addr, > > > unsigned long *image_size, > > > unsigned long *reserve_addr, > > > @@ -84,15 +72,24 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, > > > /* > > > * If KASLR is enabled, and we have some randomness available, > > > * locate the kernel at a randomized offset in physical memory. > > > + * > > > + * In that case, we don't need to preserve the 2M alignment > > > */ > > > - status = efi_random_alloc(*reserve_size, min_kimg_align(), > > > + status = efi_random_alloc(*reserve_size, EFI_KIMG_ALIGN, > > > reserve_addr, phys_seed); > > > } else { > > > status = EFI_OUT_OF_RESOURCES; > > > } > > > > > > if (status != EFI_SUCCESS) { > > > - if (IS_ALIGNED((u64)_text, min_kimg_align())) { > > > + /* > > > + * Although relocatable kernels can fix up the misalignment with respect to > > > + * MIN_KIMG_ALIGN, the resulting virtual text addresses are subtly out of > > > + * sync with those recorded in the vmlinux when kaslr is disabled but the > > > + * image required relocation anyway. Therefore retain 2M alignment unless > > > + * KASLR is in use. > > > + */ > > > + if (IS_ALIGNED((u64)_text, MIN_KIMG_ALIGN)) { > > > /* > > > * Just execute from wherever we were loaded by the > > > * UEFI PE/COFF loader if the alignment is suitable. > > > @@ -103,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, > > > } > > > > > > status = efi_allocate_pages_aligned(*reserve_size, reserve_addr, > > > - ULONG_MAX, min_kimg_align()); > > > + ULONG_MAX, MIN_KIMG_ALIGN); > > > > > > if (status != EFI_SUCCESS) { > > > efi_err("Failed to relocate kernel\n"); > > > > > > > > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Tue, 2021-07-20 at 15:48 +0200, Ard Biesheuvel wrote: > > You are replacing min_kimg_align() with MIN_KIMG_ALIGN in a place > where it could return either value: efi_nokaslr will be false by > default on relocatable kernels Not exactly: drivers/firmware/efi/libstub/efi-stub-helper.c:bool efi_nokaslr = !IS_ENABLED(CONFIG_RANDOMIZE_BASE); So if CONFIG_RANDOMIZE_BASE is off (KASLR disabled in the config), efi_nokaslr is true. If CONFIG_RANDOMIZE_BASE is on, then it depends on the command line (and the availability of the RNG protocol). None of this depends on CONFIG_RELOCATABLE which is indeed not entirely orthogonal, but not particularily relevant in how the code is written today. > , in which case min_kimg_align() will > return EFI_KIMG_ALIGN, unless you specifically request KASLR to be > disabled. Nope. See above. It will only be EFI_KIMG_ALIGN if CONFIG_RANDOMIZE_BASE is on and KASLR isn't otherwise disabled. > The result is that relocatable kernels that would not require to be > moved will now be moved to a 2 MB aligned offset before booting them. > > Similarly for the efi_allocate_pages_aligned() call: that call would > only request 64k alignment before on a relocatable kernel if booting > without randomization. I'm not sure I'm following you here. If you look at the changelog for commit 7c116db24d94, it pretty clearly says: "Adjust the EFI stub for arm64 so that the minimum Image alignment is 2MB unless KASLR is in use." Which is also pretty much what is spelled in the comment above min_kimg_align() (which I moved but kept in my patch). Basically, what you describe is what the code used to do afaik, but not what it does since 7c116db24d94. The current code (prior) to my patch is pretty clear, it uses 64k alignment if KASLR is on, otherwise 2MB. So the big if (status != EFI_SUCCESS) statement with the alignment check & relocation is all only meant to be used in the !KASLR case, which is always going to want 2MB (again based on the code as written today). My patch simply ensures that this is also true when KASLR fails to randomize the kernel address. Cheers, Ben.
On Tue, 20 Jul 2021 at 16:04, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > On Tue, 2021-07-20 at 15:48 +0200, Ard Biesheuvel wrote: > > > > You are replacing min_kimg_align() with MIN_KIMG_ALIGN in a place > > where it could return either value: efi_nokaslr will be false by > > default on relocatable kernels > > Not exactly: > > drivers/firmware/efi/libstub/efi-stub-helper.c:bool efi_nokaslr = > !IS_ENABLED(CONFIG_RANDOMIZE_BASE); > > So if CONFIG_RANDOMIZE_BASE is off (KASLR disabled in the config), > efi_nokaslr is true. > > If CONFIG_RANDOMIZE_BASE is on, then it depends on the command line > (and the availability of the RNG protocol). > > None of this depends on CONFIG_RELOCATABLE which is indeed not entirely > orthogonal, but not particularily relevant in how the code is written > today. > > > , in which case min_kimg_align() will > > return EFI_KIMG_ALIGN, unless you specifically request KASLR to be > > disabled. > > Nope. See above. It will only be EFI_KIMG_ALIGN if > CONFIG_RANDOMIZE_BASE is on and KASLR isn't otherwise disabled. > > > The result is that relocatable kernels that would not require to be > > moved will now be moved to a 2 MB aligned offset before booting them. > > > > Similarly for the efi_allocate_pages_aligned() call: that call would > > only request 64k alignment before on a relocatable kernel if booting > > without randomization. > > I'm not sure I'm following you here. If you look at the changelog for > commit 7c116db24d94, it pretty clearly says: > > "Adjust the EFI stub for arm64 so that the minimum Image alignment is > 2MB unless KASLR is in use." > > Which is also pretty much what is spelled in the comment > above min_kimg_align() (which I moved but kept in my patch). > > Basically, what you describe is what the code used to do afaik, but not > what it does since 7c116db24d94. > > The current code (prior) to my patch is pretty clear, it uses 64k > alignment if KASLR is on, otherwise 2MB. So the big if (status != > EFI_SUCCESS) statement with the alignment check & relocation is all > only meant to be used in the !KASLR case, which is always going to want > 2MB (again based on the code as written today). > > My patch simply ensures that this is also true when KASLR fails to > randomize the kernel address. > Fair enough. The history here is that passing nokaslr on the command line would force 2M alignment even if KASLR was not enabled to begin with, without affecting the alignment policy of KASLR capable kernels if KASLR was not explicitly disabled, but not available on the platform. I realize now that my commit d32de9130f6c7 has interfered with this: efi_nokaslr is set to true there so the later code does not complain about the EFI_RNG_PROTOCOL being unavailable, but it has the side effect of affecting the alignment policy in the remainder of the function. So what I would prefer here is to capture efi_nokaslr at entry, and use that to decide the alignment. That way, efi_nokaslr can be set to true without affecting the subsequent allocation logic.
On Tue, 2021-07-20 at 16:10 +0200, Ard Biesheuvel wrote: > > > My patch simply ensures that this is also true when KASLR fails to > > randomize the kernel address. > > > > Fair enough. > > The history here is that passing nokaslr on the command line would > force 2M alignment even if KASLR was not enabled to begin with, > without affecting the alignment policy of KASLR capable kernels if > KASLR was not explicitly disabled, but not available on the platform. > > I realize now that my commit d32de9130f6c7 has interfered with this: > efi_nokaslr is set to true there so the later code does not complain > about the EFI_RNG_PROTOCOL being unavailable, but it has the side > effect of affecting the alignment policy in the remainder of the > function. > > So what I would prefer here is to capture efi_nokaslr at entry, and > use that to decide the alignment. That way, efi_nokaslr can be set to > true without affecting the subsequent allocation logic. So interestingly, the bug I am trying to fix with this patch seems to indicate that this doesn't work (though I would need to debug further as to why) on my systems (EC2 c6g.metal instances). IE, in my case what happens is: - kernel has CONFIG_RANDOMIZE_BASE and CONFIG_RELOCATABLE both enabled - RNG protocol exists, it gets a random seed, but due to the other bug, efi_random_alloc() fails. It thus falls back to the "normal" alignment check & relocation case (the big if (status != EFI_SUCCESS)) - That alignment check uses 64K and not 2M however. In my case it passes (_text is already 64K aligned) and we boot... - And nothing happens. IE, it dies somewhere in/after exit boot services, I haven't had a chance to figure out in more details why, those machines take 20mn to reboot. So *something* is wrong when we stick to a 64K alignment and don't randomize the kernel base. Interestingly we don't set nokaslr in that case, so we should still be hitting all the kaslr path in the main kernel. The problem could relate to running from that initial address. I'll have to debug further, maybe try a repro-case in qemu. In the meantime, please apply patch 1 which solves the main issue and I'll continue digging. Note (in case this is relevant): This was all tested/debugged on 5.10.y Cheers, Ben.
On Tue, 20 Jul 2021 at 16:25, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > On Tue, 2021-07-20 at 16:10 +0200, Ard Biesheuvel wrote: > > > > > My patch simply ensures that this is also true when KASLR fails to > > > randomize the kernel address. > > > > > > > Fair enough. > > > > The history here is that passing nokaslr on the command line would > > force 2M alignment even if KASLR was not enabled to begin with, > > without affecting the alignment policy of KASLR capable kernels if > > KASLR was not explicitly disabled, but not available on the platform. > > > > I realize now that my commit d32de9130f6c7 has interfered with this: > > efi_nokaslr is set to true there so the later code does not complain > > about the EFI_RNG_PROTOCOL being unavailable, but it has the side > > effect of affecting the alignment policy in the remainder of the > > function. > > > > So what I would prefer here is to capture efi_nokaslr at entry, and > > use that to decide the alignment. That way, efi_nokaslr can be set to > > true without affecting the subsequent allocation logic. > > So interestingly, the bug I am trying to fix with this patch seems to > indicate that this doesn't work (though I would need to debug further > as to why) on my systems (EC2 c6g.metal instances). > > IE, in my case what happens is: > > - kernel has CONFIG_RANDOMIZE_BASE and CONFIG_RELOCATABLE both enabled > - RNG protocol exists, it gets a random seed, but due to the other > bug, efi_random_alloc() fails. It thus falls back to the "normal" > alignment check & relocation case (the big if (status != EFI_SUCCESS)) > - That alignment check uses 64K and not 2M however. In my case it > passes (_text is already 64K aligned) and we boot... > - And nothing happens. IE, it dies somewhere in/after exit boot > services, I haven't had a chance to figure out in more details why, > those machines take 20mn to reboot. > > So *something* is wrong when we stick to a 64K alignment and don't > randomize the kernel base. > Interesting. Could this be related to caching? If EFI loads Image and it remains in place, we boot the image that we are executing from, and if your ExitBootServices() implementation plays tricks with [non-architected] caches, this could cause problems, I suppose. Just guessing ... > Interestingly we don't set nokaslr in that case, so we should still be > hitting all the kaslr path in the main kernel. The problem could relate > to running from that initial address. I'll have to debug further, maybe > try a repro-case in qemu. > > In the meantime, please apply patch 1 which solves the main issue and > I'll continue digging. > OK > Note (in case this is relevant): This was all tested/debugged on 5.10.y > > Cheers, > Ben. > >
diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c index 7bf0a7acae5e..e264ff90ba03 100644 --- a/drivers/firmware/efi/libstub/arm64-stub.c +++ b/drivers/firmware/efi/libstub/arm64-stub.c @@ -34,18 +34,6 @@ efi_status_t check_platform_features(void) return EFI_SUCCESS; } -/* - * Although relocatable kernels can fix up the misalignment with respect to - * MIN_KIMG_ALIGN, the resulting virtual text addresses are subtly out of - * sync with those recorded in the vmlinux when kaslr is disabled but the - * image required relocation anyway. Therefore retain 2M alignment unless - * KASLR is in use. - */ -static u64 min_kimg_align(void) -{ - return efi_nokaslr ? MIN_KIMG_ALIGN : EFI_KIMG_ALIGN; -} - efi_status_t handle_kernel_image(unsigned long *image_addr, unsigned long *image_size, unsigned long *reserve_addr, @@ -84,15 +72,24 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, /* * If KASLR is enabled, and we have some randomness available, * locate the kernel at a randomized offset in physical memory. + * + * In that case, we don't need to preserve the 2M alignment */ - status = efi_random_alloc(*reserve_size, min_kimg_align(), + status = efi_random_alloc(*reserve_size, EFI_KIMG_ALIGN, reserve_addr, phys_seed); } else { status = EFI_OUT_OF_RESOURCES; } if (status != EFI_SUCCESS) { - if (IS_ALIGNED((u64)_text, min_kimg_align())) { + /* + * Although relocatable kernels can fix up the misalignment with respect to + * MIN_KIMG_ALIGN, the resulting virtual text addresses are subtly out of + * sync with those recorded in the vmlinux when kaslr is disabled but the + * image required relocation anyway. Therefore retain 2M alignment unless + * KASLR is in use. + */ + if (IS_ALIGNED((u64)_text, MIN_KIMG_ALIGN)) { /* * Just execute from wherever we were loaded by the * UEFI PE/COFF loader if the alignment is suitable. @@ -103,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, } status = efi_allocate_pages_aligned(*reserve_size, reserve_addr, - ULONG_MAX, min_kimg_align()); + ULONG_MAX, MIN_KIMG_ALIGN); if (status != EFI_SUCCESS) { efi_err("Failed to relocate kernel\n");
If efi_random_alloc() fails, we still try to use EFI_KIMG_ALIGN instead of MIN_KIMG_ALIGN to check the kernel image alignment, which is incorrect, we need to fallback to MIN_KIMG_ALIGN (2M). This removes the not-that-useful min_kimg_align helper and instead uses the appropriate aligment in the respective call sites: efi_random_alloc() always wants EFI_KIMG_ALIGN as this is only used when kaslr is on, and all other cases go into alignment check code which always need to check (and enforce) MIN_KIMG_ALIGN Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Fixes: 7c116db24d94 (efi/libstub/arm64: Retain 2MB kernel Image alignment if !KASLR) --- drivers/firmware/efi/libstub/arm64-stub.c | 27 ++++++++++------------- 1 file changed, 12 insertions(+), 15 deletions(-)