Re: [PATCH] x86/cet: Use dedicated NOP4 for cf_clobber

["Doebel, Bjoern" <doebel@xxxxxxxxx>]

On 08.03.22 15:01, Andrew Cooper wrote:
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open attachments unless you can confirm the sender and know the 
> content is safe.
>
>
>
> For livepatching, we need to look at a potentially clobbered function and
> determine whether it used to have an ENDBR64 instruction.
>
> Use a non-default 4-byte P6 long nop, not emitted by toolchains, and introduce
> the was_endbr64() predicate.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Bjoern Doebel <doebel@xxxxxxxxx>
> CC: Michael Kurth <mku@xxxxxxxxx>
> CC: Martin Pohlack <mpohlack@xxxxxxxxx>
>
> Bjoern: For the livepatching code, I think you want:
>
>    if ( is_endbr64(...) || was_endbr64(...) )
>        needed += ENDBR64_LEN;
> ---
>   xen/arch/x86/alternative.c       | 10 +++++++++-
>   xen/arch/x86/include/asm/endbr.h | 12 ++++++++++++
>   2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
> index d41eeef1bcaf..ffb1b1d960c8 100644
> --- a/xen/arch/x86/alternative.c
> +++ b/xen/arch/x86/alternative.c
> @@ -362,7 +362,15 @@ static void init_or_livepatch _apply_alternatives(struct 
> alt_instr *start,
>               if ( !is_kernel_text(ptr) || !is_endbr64(ptr) )
>                   continue;
>
> -            add_nops(ptr, ENDBR64_LEN);
> +            /*
> +             * Can't use add_nops() here.  ENDBR64_POISON is specifically
> +             * different to NOP4 so it can be spotted after the fact.
> +             *
> +             * All CET-capable hardware uses P6 NOPS (no need to plumb through
> +             * ideal_nops), and doesn't require a branch to synchronise the
> +             * instruction stream.
> +             */
> +            memcpy(ptr, ENDBR64_POISON, ENDBR64_LEN);
>               clobbered++;
>           }
>
> diff --git a/xen/arch/x86/include/asm/endbr.h b/xen/arch/x86/include/asm/endbr.h
> index 6090afeb0bd8..5e1e55cb467d 100644
> --- a/xen/arch/x86/include/asm/endbr.h
> +++ b/xen/arch/x86/include/asm/endbr.h
> @@ -52,4 +52,16 @@ static inline void place_endbr64(void *ptr)
>       *(uint32_t *)ptr = gen_endbr64();
>   }
>
> +/*
> + * After clobbering ENDBR64, we may need to confirm that the site used to
> + * contain an ENDBR64 instruction.  Use an encoding which isn't the default
> + * P6_NOP4.
> + */
> +#define ENDBR64_POISON "\x66\x0f\x1f\x00" /* osp nopl (%rax) */
> +
> +static inline bool was_endbr64(const void *ptr)
> +{
> +    return *(const uint32_t *)ptr == 0x001f0f66;
> +}
> +
>   #endif /* XEN_ASM_ENDBR_H */
> --
> 2.11.0
Reviewed-by: Bjoern Doebel <doebel@xxxxxxxxx>



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879