On Tue, 20 Jul 2021 at 16:04, Benjamin Herrenschmidt
[off-list ref] wrote:

On Tue, 2021-07-20 at 15:48 +0200, Ard Biesheuvel wrote:

quoted

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.

quoted

, 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.

quoted

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel