| Message ID | 20180504060003.19618-18-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | EFI updates for v4.18 | expand |
* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 14 May 2018 at 08:47, Ingo Molnar <mingo@kernel.org> wrote: > > > > * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > > >> From: Mark Rutland <mark.rutland@arm.com> > >> > >> When CONFIG_RANDOMIZE_TEXT_OFFSET is selected, TEXT_OFFSET is an > >> arbitrary multiple of PAGE_SIZE in the interval [0, 2MB). > >> > >> The EFI stub does not account for the potential misalignment of > >> TEXT_OFFSET relative to EFI_KIMG_ALIGN, and produces a randomized > >> physical offset which is always a round multiple of EFI_KIMG_ALIGN. > >> This may result in statically allocated objects whose alignment exceeds > >> PAGE_SIZE to appear misaligned in memory. This has been observed to > >> result in spurious stack overflow reports and failure to make use of > >> the IRQ stacks, and theoretically could result in a number of other > >> issues. > >> > >> We can OR in the low bits of TEXT_OFFSET to ensure that we have the > >> necessary offset (and hence preserve the misalignment of TEXT_OFFSET > >> relative to EFI_KIMG_ALIGN), so let's do that. > >> > >> Fixes: 6f26b3671184c36d ("arm64: kaslr: increase randomization granularity") > >> Cc: <stable@vger.kernel.org> # v4.7+ > >> Reported-by: Kim Phillips <kim.phillips@arm.com> > >> Signed-off-by: Mark Rutland <mark.rutland@arm.com> > >> Tested-by: Kim Phillips <kim.phillips@arm.com> > >> [ardb: clarify commit log] > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> --- > >> drivers/firmware/efi/libstub/arm64-stub.c | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c > >> index b9bd827caa22..541b82fdc8a2 100644 > >> --- a/drivers/firmware/efi/libstub/arm64-stub.c > >> +++ b/drivers/firmware/efi/libstub/arm64-stub.c > >> @@ -97,6 +97,13 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg, > >> u32 offset = !IS_ENABLED(CONFIG_DEBUG_ALIGN_RODATA) ? > >> (phys_seed >> 32) & mask : TEXT_OFFSET; > >> > >> + /* > >> + * With CONFIG_RANDOMIZE_TEXT_OFFSET, TEXT_OFFSET may not be a > >> + * multiple of EFI_KIMG_ALIGN, and we must ensure that we apply > >> + * the offset below EFI_KIMG_ALIGN. > >> + */ > > > > When referring to config variables in comments and changelogs I'd suggest a bit > > more verbosity: > > > > s/CONFIG_RANDOMIZE_TEXT_OFFSET > > /CONFIG_RANDOMIZE_TEXT_OFFSET=y > > > > ... because at first I thought (based on the name) that > > CONFIG_RANDOMIZE_TEXT_OFFSET is an actual integer offset value - while it's a > > bool. The =y makes the bool nature obvious. > > > > ( Similarly, when negated the canonical way to refer to it is > > !CONFIG_RANDOMIZE_TEXT_OFFSET. ) > > > > Fair enough. > > >> + offset |= (TEXT_OFFSET % EFI_KIMG_ALIGN); > > > > The parentheses are not needed here I think. > > > > Nope. > > Will you fix this up when applying? Or should I resend? Since this was at the tail with no dependencies I'll skip this for now I think - mind sending the refreshed version in the next batch? Thanks, Ingo
diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c index b9bd827caa22..541b82fdc8a2 100644 --- a/drivers/firmware/efi/libstub/arm64-stub.c +++ b/drivers/firmware/efi/libstub/arm64-stub.c @@ -97,6 +97,13 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table_arg, u32 offset = !IS_ENABLED(CONFIG_DEBUG_ALIGN_RODATA) ? (phys_seed >> 32) & mask : TEXT_OFFSET; + /* + * With CONFIG_RANDOMIZE_TEXT_OFFSET, TEXT_OFFSET may not be a + * multiple of EFI_KIMG_ALIGN, and we must ensure that we apply + * the offset below EFI_KIMG_ALIGN. + */ + offset |= (TEXT_OFFSET % EFI_KIMG_ALIGN); + /* * If KASLR is enabled, and we have some randomness available, * locate the kernel at a randomized offset in physical memory.