> Date: Mon, 19 Oct 2020 16:36:14 +0200
> From: Christian Weisgerber <[email protected]>
> 
> Belatedly, ARM has taken a slice of the reserved opcode space and
> assigned it as a properly defined illegal instruction, udf #imm16.
> (Armv8 Architecture Reference Manual, edition F.c, section C6.2.335).
> Clang already knows about it.
> 
> We really should use this instead of picking something ad-hoc out
> of the opcode space.
> 
> I have verified that this builds on arm64, produces a SIGILL in
> userland, and drops me into ddb in the kernel.
> 
> armv7 has an equivalent instruction.  kettenis@ confirms it builds
> and SIGILLs there.
> 
> OK?

So on armv7 there is an additional consideration.  The architecture
defines tow instruction sets: A32 and T32 (Thumb).  A32 instructions
are 32-bit but T32 instructions can be 16-bit.  If an attacker can
switch the CPU into T32 mode, it will interpret this UDF instruction
as two different instructions.  We may have to consider how "bad"
these two instructions are and maybe tune that #imm16 accordingly.

> Index: lib/csu/aarch64/md_init.h
> ===================================================================
> RCS file: /cvs/src/lib/csu/aarch64/md_init.h,v
> retrieving revision 1.9
> diff -u -p -r1.9 md_init.h
> --- lib/csu/aarch64/md_init.h 15 Oct 2020 16:30:21 -0000      1.9
> +++ lib/csu/aarch64/md_init.h 19 Oct 2020 11:57:02 -0000
> @@ -115,5 +115,5 @@
>       "       svc     #0                      \n" \
>       "       dsb     nsh                     \n" \
>       "       isb                             \n" \
> -     "       .word 0xa000f7f0 /* illegal */  \n" \
> +     "       udf     #0                      \n" \
>       ".previous");
> Index: lib/csu/arm/md_init.h
> ===================================================================
> RCS file: /cvs/src/lib/csu/arm/md_init.h,v
> retrieving revision 1.16
> diff -u -p -r1.16 md_init.h
> --- lib/csu/arm/md_init.h     15 Oct 2020 16:30:23 -0000      1.16
> +++ lib/csu/arm/md_init.h     19 Oct 2020 13:23:00 -0000
> @@ -159,5 +159,5 @@
>       "       swi     #0                      \n" \
>       "       dsb     nsh                     \n" \
>       "       isb                             \n" \
> -     "       .word 0xa000f7f0 /* illegal */  \n" \
> +     "       udf     #0                      \n" \
>       ".previous");
> Index: lib/libc/arch/aarch64/sys/tfork_thread.S
> ===================================================================
> RCS file: /cvs/src/lib/libc/arch/aarch64/sys/tfork_thread.S,v
> retrieving revision 1.5
> diff -u -p -r1.5 tfork_thread.S
> --- lib/libc/arch/aarch64/sys/tfork_thread.S  18 Oct 2020 14:28:16 -0000      
> 1.5
> +++ lib/libc/arch/aarch64/sys/tfork_thread.S  19 Oct 2020 11:59:32 -0000
> @@ -43,6 +43,6 @@ ENTRY(__tfork_thread)
>       mov     x0, x3
>       blr     x2
>       SYSTRAP(__threxit)
> -     .word   0xa000f7f0      /* illegal on all cpus? */
> +     udf     #0
>       .cfi_endproc
>  END(__tfork_thread)
> Index: lib/libc/arch/arm/sys/tfork_thread.S
> ===================================================================
> RCS file: /cvs/src/lib/libc/arch/arm/sys/tfork_thread.S,v
> retrieving revision 1.5
> diff -u -p -r1.5 tfork_thread.S
> --- lib/libc/arch/arm/sys/tfork_thread.S      18 Oct 2020 14:28:17 -0000      
> 1.5
> +++ lib/libc/arch/arm/sys/tfork_thread.S      19 Oct 2020 13:23:35 -0000
> @@ -37,5 +37,5 @@ ENTRY(__tfork_thread)
>       mov     pc, r2
>       nop
>       SYSTRAP(__threxit)
> -     .word   0xa000f7f0      /* illegal on all cpus? */
> +     udf     #0
>  END(__tfork_thread)
> Index: sys/arch/arm/arm/sigcode.S
> ===================================================================
> RCS file: /cvs/src/sys/arch/arm/arm/sigcode.S,v
> retrieving revision 1.9
> diff -u -p -r1.9 sigcode.S
> --- sys/arch/arm/arm/sigcode.S        13 Mar 2020 08:46:50 -0000      1.9
> +++ sys/arch/arm/arm/sigcode.S        19 Oct 2020 13:23:55 -0000
> @@ -72,7 +72,7 @@ _C_LABEL(esigcode):
>  
>       .globl  sigfill
>  sigfill:
> -     .word   0xa000f7f0              /* illegal on all cpus? */
> +     udf     #0
>  esigfill:
>  
>       .data
> Index: sys/arch/arm64/arm64/locore.S
> ===================================================================
> RCS file: /cvs/src/sys/arch/arm64/arm64/locore.S,v
> retrieving revision 1.31
> diff -u -p -r1.31 locore.S
> --- sys/arch/arm64/arm64/locore.S     13 Mar 2020 00:14:38 -0000      1.31
> +++ sys/arch/arm64/arm64/locore.S     19 Oct 2020 12:02:23 -0000
> @@ -366,7 +366,7 @@ _C_LABEL(esigcode):
>  
>       .globl  sigfill
>  sigfill:
> -     .word   0xa000f7f0              /* FIXME: illegal on all cpus? */
> +     udf     #0
>  esigfill:
>  
>       .data
> -- 
> Christian "naddy" Weisgerber                          [email protected]
> 
> 

Reply via email to