[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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





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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.