diff mbox series

[6/9] xen/arm64: entry: Don't jump outside of an alternative

Message ID 20230625204907.57291-7-julien@xen.org (mailing list archive)
State New, archived
Headers show
Series xen/arm: Enable UBSAN support | expand

Commit Message

Julien Grall June 25, 2023, 8:49 p.m. UTC
From: Julien Grall <jgrall@amazon.com>

The instruction CBNZ can only jump to a pc-relative that is in the
range +/- 1MB.

Alternative instructions replacement are living in a separate
subsection of the init section. This is usually placed towards
the end of the linker. Whereas text is towards the beginning.

While today Xen is quite small (~1MB), it could grow up to
2MB in the current setup. So there is no guarantee that the
target address in the text section will be within the range +/-
1MB of the CBNZ in alternative section.

The easiest solution is to have the target address within the
same section of the alternative. This means that we need to
duplicate a couple of instructions.

Signed-off-by: Julien Grall <jgrall@amazon.com>

----

I couldn't come up with a solution that would not change the number
of instructions executed in the entry path.
---
 xen/arch/arm/arm64/entry.S | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

Comments

Michal Orzel June 27, 2023, 7:52 a.m. UTC | #1
On 25/06/2023 22:49, Julien Grall wrote:
> 
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> The instruction CBNZ can only jump to a pc-relative that is in the
> range +/- 1MB.
> 
> Alternative instructions replacement are living in a separate
> subsection of the init section. This is usually placed towards
> the end of the linker. Whereas text is towards the beginning.
> 
> While today Xen is quite small (~1MB), it could grow up to
> 2MB in the current setup. So there is no guarantee that the
> target address in the text section will be within the range +/-
> 1MB of the CBNZ in alternative section.
> 
> The easiest solution is to have the target address within the
> same section of the alternative. This means that we need to
> duplicate a couple of instructions.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> ----
> 
> I couldn't come up with a solution that would not change the number
> of instructions executed in the entry path.
It looks like the max offset is indeed 1MB for conditional branches and I cannot
think of any better way of doing this, so:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

> ---
>  xen/arch/arm/arm64/entry.S | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 95f1a9268419..492591fdef54 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -242,13 +242,24 @@
>          msr     daifclr, \iflags
>          bl      enter_hypervisor_from_guest
> 
> +        /*
> +         * CBNZ can only address an offset of +/- 1MB. This means, it is
> +         * not possible to jump outside of an alternative because
> +         * the .text section and .altinstr_replacement may be further
> +         * appart. The easiest way is to duplicate the few instructions
s/appart/apart

~Michal
diff mbox series

Patch

diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 95f1a9268419..492591fdef54 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -242,13 +242,24 @@ 
         msr     daifclr, \iflags
         bl      enter_hypervisor_from_guest
 
+        /*
+         * CBNZ can only address an offset of +/- 1MB. This means, it is
+         * not possible to jump outside of an alternative because
+         * the .text section and .altinstr_replacement may be further
+         * appart. The easiest way is to duplicate the few instructions
+         * that need to be skipped.
+         */
         alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
-        cbnz    x19, 1f
-        alternative_else_nop_endif
-
-        mov     x0, sp
-        bl      do_trap_\trap
+        cbnz      x19, 1f
+        mov       x0, sp
+        bl        do_trap_\trap
 1:
+        alternative_else
+        nop
+        mov       x0, sp
+        bl        do_trap_\trap
+        alternative_endif
+
         exit    hyp=0, compat=\compat
         .endm