2023-06-08 14:08:17

by Jürgen Groß

[permalink] [raw]
Subject: [RFC PATCH 2/3] x86/alternative: add indirect call patching

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 <[email protected]>
---
arch/x86/include/asm/alternative.h | 5 ++++
arch/x86/kernel/alternative.c | 37 +++++++++++++++++++++++++++---
2 files changed, 39 insertions(+), 3 deletions(-)

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++)
--
2.35.3



2023-09-20 18:40:40

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC PATCH 2/3] x86/alternative: add indirect call patching

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 <[email protected]>

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;