[PATCH] arm64/alternatives: use subsections for replacement sequences

[Dave P Martin]

On Tue, Jun 30, 2020 at 10:19:21AM +0200, Ard Biesheuvel wrote:
> When building very large kernels, the logic that emits replacement
> sequences for alternatives fails when relative branches are present
> in the code that is emitted into the .altinstr_replacement section
> and patched in at the original site and fixed up. The reason is that
> the linker will insert veneers if relative branches go out of range,
> and due to the relative distance of the .altinstr_replacement from
> the .text section where its branch targets usually live, veneers
> may be emitted at the end of the .altinstr_replacement section, with
> the relative branches in the sequence pointed at the veneers instead
> of the actual target.
> 
> The alternatives patching logic will attempt to fix up the branch to
> point to its original target, which will be the veneer in this case,
> but given that the patch site is likely to be far away as well, it
> will be out of range and so patching will fail. There are other cases
> where these veneers are problematic, e.g., when the target of the
> branch is in .text while the patch site is in .init.text, in which
> case putting the replacement sequence inside .text may not help either.
> 
> So let's use subsections to emit the replacement code as closely as
> possible to the patch site, to ensure that veneers are only likely to
> be emitted if they are required at the patch site as well, in which
> case they will be in range for the replacement sequence both before
> and after it is transported to the patch site.
> 
> This will prevent alternative sequences in non-init code from being
> released from memory after boot, but this is tolerable given that the
> entire section is only 512 KB on an allyesconfig build (which weighs in
> at 500+ MB for the entire Image). Also, note that modules today carry
> the replacement sequences in non-init sections as well, and any of
> those that target init code will be emitted into init sections after
> this change.
> 
> This fixes an early crash when booting an allyesconfig kernel on a
> system where any of the alternatives sequences containing relative
> branches are activated at boot (e.g., ARM64_HAS_PAN on TX2)
> 
> Cc: Suzuki K Poulose <suzuki.poulose at arm.com>
> Cc: James Morse <james.morse at arm.com>
> Cc: Andre Przywara <andre.przywara at arm.com>
> Cc: Dave P Martin <dave.martin at arm.com>
> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> ---
>  arch/arm64/include/asm/alternative.h | 16 ++++++++--------
>  arch/arm64/kernel/vmlinux.lds.S      |  3 ---
>  2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index 5e5dc05d63a0..12f0eb56a1cc 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -73,11 +73,11 @@ static inline void apply_alternatives_module(void *start, size_t length) { }
>  	".pushsection .altinstructions,\"a\"\n"				\
>  	ALTINSTR_ENTRY(feature)						\
>  	".popsection\n"							\
> -	".pushsection .altinstr_replacement, \"a\"\n"			\
> +	".subsection 1\n"						\

This uses subsections in existing sections.  Could that interfere with
existing (or future) uses of subsections?  (I've not checked whether
there actually are such uses.  I'm also assuming that clobbering the
invoker's idea of what section is .previous doesn't matter.)

Another wrinkle: the replacement code now becomes executable, whereas
I think it was previously in rodata.  I'm not sure how much this
matters, but it might be a source of gadgets.


A different option would be to add an explicitly veneered branch macro
for use in alternatives, maybe adrp+add+br.  For BTI compatility, we'd
need a bti j or equivalent at the destination, which might or might not
be easy to achieve -- mind you, I think we theoretically need that
anyway for veneers to work properly in all cases.

Because we would define the exact instruction sequence, the
alternatives code could probably replace it with a direct branch if the
actual destination is close enough.  The downside is that it wouldn't
be a single instruction any more, and there would be some overhead for
conditional branches if we replace the unneeded insns with NOPs.

[...]

Cheers
---Dave