Re: [PATCH 1/3] x86/alternatives: Teach text_poke_bp() to emulate instructions

[Peter Zijlstra]

On Thu, Oct 03, 2019 at 01:01:06PM +0200, Peter Zijlstra wrote:
> Also, I think text_poke_bp(INT3) is broken, although I don't think
> anybody actually does that. Still, let me fix that.

Something like so should allow text_poke_bp(INT3) to work as expected.

---
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -999,6 +999,13 @@ int poke_int3_handler(struct pt_regs *re
        ip += tp->len;
 
        switch (tp->opcode) {
+       case INT3_INSN_OPCODE:
+               /*
+                * Someone poked an explicit INT3, they'll want to handle it,
+                * do not consume.
+                */
+               return 0;
+
        case CALL_INSN_OPCODE:
                int3_emulate_call(regs, (long)ip + tp->rel32);
                break;
@@ -1040,8 +1047,8 @@ NOKPROBE_SYMBOL(poke_int3_handler);
 void text_poke_bp_batch(struct text_poke_loc *tp, unsigned int nr_entries)
 {
        unsigned char int3 = INT3_INSN_OPCODE;
-       int patched_all_but_first = 0;
        unsigned int i;
+       int do_sync;
 
        lockdep_assert_held(&text_mutex);
 
@@ -1065,16 +1072,16 @@ void text_poke_bp_batch(struct text_poke
        /*
         * Second step: update all but the first byte of the patched range.
         */
-       for (i = 0; i < nr_entries; i++) {
+       for (do_sync = 0, i = 0; i < nr_entries; i++) {
                if (tp[i].len - sizeof(int3) > 0) {
                        text_poke((char *)tp[i].addr + sizeof(int3),
                                  (const char *)tp[i].text + sizeof(int3),
                                  tp[i].len - sizeof(int3));
-                       patched_all_but_first++;
+                       do_sync++;
                }
        }
 
-       if (patched_all_but_first) {
+       if (do_sync) {
                /*
                 * According to Intel, this core syncing is very likely
                 * not necessary and we'd be safe even without it. But
@@ -1087,10 +1094,17 @@ void text_poke_bp_batch(struct text_poke
         * Third step: replace the first byte (int3) by the first byte of
         * replacing opcode.
         */
-       for (i = 0; i < nr_entries; i++)
+       for (do_sync = 0, i = 0; i < nr_entries; i++) {
+               if (tp[i].text[0] == INT3_INSN_OPCODE)
+                       continue;
+
                text_poke(tp[i].addr, tp[i].text, sizeof(int3));
+               do_sync++;
+       }
+
+       if (do_sync)
+               on_each_cpu(do_sync_core, NULL, 1);
 
-       on_each_cpu(do_sync_core, NULL, 1);
        /*
         * sync_core() implies an smp_mb() and orders this store against
         * the writing of the new instruction.
@@ -1123,6 +1137,9 @@ void text_poke_loc_init(struct text_poke
        tp->opcode = insn.opcode.bytes[0];
 
        switch (tp->opcode) {
+       case INT3_INSN_OPCPDE:
+               break;
+
        case CALL_INSN_OPCODE:
        case JMP32_INSN_OPCODE:
        case JMP8_INSN_OPCODE: