Re: [PATCH v2 64/70] x86: Introduce helpers/checks for endbr64 instructions

[Jan Beulich]

On 14.02.2022 13:51, Andrew Cooper wrote:
> ... to prevent the optimiser creating unsafe code.  See the code comment for
> full details.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 

Re: [PATCH v2 64/70] x86: Introduce helpers/checks for endbr64 instructions

[Andrew Cooper]

On 14/02/2022 12:51, Andrew Cooper wrote:
> ... to prevent the optimiser creating unsafe code.  See the code comment for
> full details.
>
> Signed-off-by: Andrew Cooper 

From review in the follow-up series, I've merged this delta:

diff --git a/xen/arch/x86/include/asm/endbr.h
b/xen/arch/x86/include/asm/endbr.h
index 6b6f46afaf29..6090afeb0bd8 100644
--- a/xen/arch/x86/include/asm/endbr.h
+++ b/xen/arch/x86/include/asm/endbr.h
@@ -19,6 +19,8 @@
 
 #include 
 
+#define ENDBR64_LEN 4
+
 /*
  * In some cases we need to inspect/insert endbr64 instructions.
  *

in, to replace some raw 4's.

~Andrew

[PATCH v2 64/70] x86: Introduce helpers/checks for endbr64 instructions

[Andrew Cooper]

... to prevent the optimiser creating unsafe code.  See the code comment for
full details.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 

v2:
 * Fix include to let the header be standalone
 * Add earlyclobber to asm
v1.1:
 * New
---
 xen/arch/x86/include/asm/endbr.h | 53 
 1 file changed, 53 insertions(+)
 create mode 100644 xen/arch/x86/include/asm/endbr.h

diff --git a/xen/arch/x86/include/asm/endbr.h b/xen/arch/x86/include/asm/endbr.h
new file mode 100644
index ..6b6f46afaf29
--- /dev/null
+++ b/xen/arch/x86/include/asm/endbr.h
@@ -0,0 +1,53 @@
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; If not, see .
+ *
+ * Copyright (c) 2021-2022 Citrix Systems Ltd.
+ */
+#ifndef XEN_ASM_ENDBR_H
+#define XEN_ASM_ENDBR_H
+
+#include 
+
+/*
+ * In some cases we need to inspect/insert endbr64 instructions.
+ *
+ * The naive way, mem{cmp,cpy}(ptr, "\xf3\x0f\x1e\xfa", 4), optimises unsafely
+ * by placing 0xfa1e0ff3 in an imm32 operand, and marks a legal indirect
+ * branch target as far as the CPU is concerned.
+ *
+ * gen_endbr64() is written deliberately to avoid the problematic operand, and
+ * marked __const__ as it is safe for the optimiser to hoist/merge/etc.
+ */
+static inline uint32_t __attribute_const__ gen_endbr64(void)
+{
+uint32_t res;
+
+asm ( "mov $~0xfa1e0ff3, %[res]\n\t"
+  "not %[res]\n\t"
+  : [res] "=" (res) );
+
+return res;
+}
+
+static inline bool is_endbr64(const void *ptr)
+{
+return *(const uint32_t *)ptr == gen_endbr64();
+}
+
+static inline void place_endbr64(void *ptr)
+{
+*(uint32_t *)ptr = gen_endbr64();
+}
+
+#endif /* XEN_ASM_ENDBR_H */
-- 
2.11.0