[RFC,2/3] x86/alternative: add indirect call patching

Message ID 20230608140333.4083-3-jgross@suse.com
State New
Headers
Series x86/paravirt: Get rid of paravirt patching |

Commit Message

Juergen Gross June 8, 2023, 2:03 p.m. UTC
  In order to prepare replacing of paravirt patching with alternative
patching, add the capability to replace an indirect call with a direct
one to alternative patching.

This is done via a new flag ALT_FLAG_CALL as the target of the call
instruction needs to be evaluated using the value of the location
addressed by the indirect call. For convenience add a macro for a
default call instruction. In case it is being used without the new
flag being set, it will result in a BUG() when being executed. As in
most cases the feature used will be X86_FEATURE_ALWAYS add another
macro ALT_CALL_ALWAYS usable for the flags parameter of the ALTERNATIVE
macros.

For a complete replacement handle the special cases of calling a nop
function and an indirect call of NULL the same way as paravirt does.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/include/asm/alternative.h |  5 ++++
 arch/x86/kernel/alternative.c      | 37 +++++++++++++++++++++++++++---
 2 files changed, 39 insertions(+), 3 deletions(-)
  

Comments

Peter Zijlstra Sept. 20, 2023, 2:44 p.m. UTC | #1
On Thu, Jun 08, 2023 at 04:03:32PM +0200, Juergen Gross wrote:
> In order to prepare replacing of paravirt patching with alternative
> patching, add the capability to replace an indirect call with a direct
> one to alternative patching.
> 
> This is done via a new flag ALT_FLAG_CALL as the target of the call
> instruction needs to be evaluated using the value of the location
> addressed by the indirect call. For convenience add a macro for a
> default call instruction. In case it is being used without the new
> flag being set, it will result in a BUG() when being executed. As in
> most cases the feature used will be X86_FEATURE_ALWAYS add another
> macro ALT_CALL_ALWAYS usable for the flags parameter of the ALTERNATIVE
> macros.
> 
> For a complete replacement handle the special cases of calling a nop
> function and an indirect call of NULL the same way as paravirt does.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Since I wrecked all that with 270a69c4485d ("x86/alternative: Support
relocations in alternatives") I wrote you a replacement :-)

---
Index: linux-2.6/arch/x86/include/asm/alternative.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/alternative.h
+++ linux-2.6/arch/x86/include/asm/alternative.h
@@ -10,6 +10,9 @@
 
 #define ALT_FLAG_NOT		(1 << 0)
 #define ALT_NOT(feature)	((ALT_FLAG_NOT << ALT_FLAGS_SHIFT) | (feature))
+#define ALT_FLAG_CALL		(1 << 1)
+#define ALT_CALL(feature)	((ALT_FLAG_CALL << ALT_FLAGS_SHIFT) | (feature))
+#define ALT_CALL_ALWAYS		ALT_CALL(X86_FEATURE_ALWAYS)
 
 #ifndef __ASSEMBLY__
 
@@ -150,6 +153,8 @@ static inline int alternatives_text_rese
 }
 #endif	/* CONFIG_SMP */
 
+#define ALT_CALL_INSTR		"call x86_BUG"
+
 #define b_replacement(num)	"664"#num
 #define e_replacement(num)	"665"#num
 
Index: linux-2.6/arch/x86/kernel/alternative.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/alternative.c
+++ linux-2.6/arch/x86/kernel/alternative.c
@@ -396,6 +396,40 @@ noinstr void x86_BUG(void)
 EXPORT_SYMBOL_GPL(x86_BUG);
 
 /*
+ * Rewrite the "call x86_BUG" replacement to point to the target of the
+ * indirect pv_ops call "call *disp(%ip)".
+ */
+static int alt_replace_call(u8 *instr, u8 *insn_buff, struct alt_instr *a)
+{
+	void *target, *bug = &x86_BUG;
+
+	if (a->replacementlen != 5 || insn_buff[0] != CALL_INSN_OPCODE) {
+		pr_err("Alternative: ALT_FLAG_CALL set for a non-call replacement instruction\n");
+		pr_err("  Ignoring the flag for the instruction at %pS (%px)\n", instr, instr);
+		return 5;
+	}
+
+	if (a->instrlen != 6 || instr[0] != 0xff || instr[1] != 0x15) {
+		pr_err("Alternative: ALT_FLAG_CALL set for unrecognized indirect call\n");
+		pr_err("  Not replacing the instruction at %pS (%px)\n", instr, instr);
+		return -1;
+	}
+
+	/* ff 15 00 00 00 00   call   *0x0(%rip) */
+	target = *(void **)(instr + a->instrlen + *(s32 *)(instr + 2));
+	if (!target)
+		target = bug;
+
+	/* (x86_BUG - .) + (target - x86_BUG) := target - . */
+	*(s32 *)(insn_buff + 1) += target - bug;
+
+	if (target == &x86_nop)
+		return 0;
+
+	return 5;
+}
+
+/*
  * Replace instructions with better alternatives for this CPU type. This runs
  * before SMP is initialized to avoid SMP problems with self modifying code.
  * This implies that asymmetric systems where APs have less capabilities than
@@ -451,6 +485,12 @@ void __init_or_module noinline apply_alt
 		memcpy(insn_buff, replacement, a->replacementlen);
 		insn_buff_sz = a->replacementlen;
 
+		if (a->flags & ALT_FLAG_CALL) {
+			insn_buff_sz = alt_replace_call(instr, insn_buff, a);
+			if (insn_buff_sz < 0)
+				continue;
+		}
+
 		for (; insn_buff_sz < a->instrlen; insn_buff_sz++)
 			insn_buff[insn_buff_sz] = 0x90;
  

Patch

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index a5a4944ce5d1..8e0871c1631a 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -10,6 +10,9 @@ 
 
 #define ALT_FLAG_NOT		(1 << 0)
 #define ALT_NOT(feature)	((ALT_FLAG_NOT << ALT_FLAGS_SHIFT) | (feature))
+#define ALT_FLAG_CALL		(1 << 1)
+#define ALT_CALL(feature)	((ALT_FLAG_CALL << ALT_FLAGS_SHIFT) | (feature))
+#define ALT_CALL_ALWAYS		ALT_CALL(X86_FEATURE_ALWAYS)
 
 #ifndef __ASSEMBLY__
 
@@ -155,6 +158,8 @@  static inline int alternatives_text_reserved(void *start, void *end)
 }
 #endif	/* CONFIG_SMP */
 
+#define ALT_CALL_INSTR		"call x86_BUG"
+
 #define b_replacement(num)	"664"#num
 #define e_replacement(num)	"665"#num
 
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index b7c70479fe58..f0371304c1b5 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -321,19 +321,50 @@  void __init_or_module noinline apply_alternatives(struct alt_instr *start,
 		insn_buff_sz = a->replacementlen;
 
 		/*
-		 * 0xe8 is a relative jump; fix the offset.
+		 * 0xe8 is a relative call; fix the offset.
 		 *
 		 * Instruction length is checked before the opcode to avoid
 		 * accessing uninitialized bytes for zero-length replacements.
 		 */
 		if (a->replacementlen == 5 && *insn_buff == 0xe8) {
-			*(s32 *)(insn_buff + 1) += replacement - instr;
+			s32 *displ = (s32 *)(insn_buff + 1);
+
+			if (a->flags & ALT_FLAG_CALL) {
+				u8 *f_ptr;
+
+				if (a->instrlen != 6 ||
+				    (instr[0] != 0xff && instr[1] != 0x15)) {
+					pr_err("Alternative: ALT_FLAG_CALL set for unrecognized indirect call\n");
+					pr_err("  Not replacing instruction at %pS (%px)\n",
+					       instr, instr);
+					continue;
+				}
+
+				f_ptr = *(u8 **)(instr + a->instrlen +
+						 *(s32 *)(instr + 2));
+
+				/* Replace calls of NULL with explicit BUG(). */
+				if (!f_ptr)
+					f_ptr = (u8 *)x86_BUG;
+
+				/* Replace a nop call with nops. */
+				if (f_ptr == (u8 *)x86_nop)
+					insn_buff_sz = 0;
+				else
+					*displ = f_ptr - (instr + insn_buff_sz);
+			} else {
+				*displ += replacement - instr;
+			}
 			DPRINTK("Fix CALL offset: 0x%x, CALL 0x%lx",
 				*(s32 *)(insn_buff + 1),
 				(unsigned long)instr + *(s32 *)(insn_buff + 1) + 5);
+		} else if (a->flags & ALT_FLAG_CALL) {
+			pr_err("Alternative: ALT_FLAG_CALL set for a non-call replacement instruction\n");
+			pr_err("  Ignoring the flag for instruction at %pS (%px)\n",
+			       instr, instr);
 		}
 
-		if (a->replacementlen && is_jmp(replacement[0]))
+		if (insn_buff_sz && is_jmp(replacement[0]))
 			recompute_jump(a, instr, replacement, insn_buff);
 
 		for (; insn_buff_sz < a->instrlen; insn_buff_sz++)