On Mon, Apr 01, 2019 at 10:58:17AM +0200, Daniel Bristot de Oliveira wrote:
> + ip = (void *) regs->ip - sizeof(unsigned char);
That one confused me mightily. Does that want to be:
ip = (void *)regs->ip - LEN_INT3;
or something? Even just a naked 1 would've been less confusing.
>
> + /*
> + * Skip the binary search if there is a single member in the vector.
> + */
> + if (unlikely(bp_patching.nr_entries == 1))
> + goto single_poke;
> +
> + tp = bsearch(ip, bp_patching.vec, bp_patching.nr_entries,
> + sizeof(struct text_patch_loc),
> + patch_cmp);
> + if (!tp)
> + return 0;
> +
> + /* set up the specified breakpoint detour */
> + regs->ip = (unsigned long) tp->detour;
> return 1;
> +single_poke:
> + if (ip == bp_patching.vec->addr) {
> + regs->ip = (unsigned long) bp_patching.vec->detour;
> + return 1;
> + }
> +
> + return 0;
if (bp_patching.nr_entries > 1) {
tp = bsearch(ip, bp_patching.vec, bp_patching.nr_entries,
sizeof(struct text_patch_loc), patch_cmp);
if (!tp)
return 0;
} else {
tp = bp_patching.vec;
if (tp->addr != ip)
return 0;
}
regs->ip = (unsigned long)tp->detour;
return 1;
> }
> NOKPROBE_SYMBOL(poke_int3_handler);
> +void text_poke_bp_batch(struct text_patch_loc *tp, unsigned int nr_entries)
> {
> + int patched_all_but_first = 0;
> unsigned char int3 = 0xcc;
> + unsigned int i;
>
> lockdep_assert_held(&text_mutex);
>
> + bp_patching.vec = tp;
> + bp_patching.nr_entries = nr_entries;
> + bp_patching.in_progress = true;
> /*
> * Corresponding read barrier in int3 notifier for making sure the
> * in_progress and handler are correctly ordered wrt. patching.
> */
> smp_wmb();
>
> + /*
> + * First step: add a int3 trap to the address that will be patched.
> + */
> + for (i = 0; i < nr_entries; i++)
> + text_poke(tp[i].addr, &int3, sizeof(int3));
>
> on_each_cpu(do_sync_core, NULL, 1);
>
> + /*
> + * Second step: update all but the first byte of the patched range.
> + */
> + for (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].opcode + sizeof(int3),
> + tp[i].len - sizeof(int3));
> + patched_all_but_first++;
> + }
> + }
> +
> + if (patched_all_but_first) {
> /*
> * According to Intel, this core syncing is very likely
> * not necessary and we'd be safe even without it. But
> @@ -821,15 +874,52 @@ void *text_poke_bp(void *addr, const void *opcode,
> size_t len, void *handler)
> on_each_cpu(do_sync_core, NULL, 1);
> }
>
> + /*
> + * Third step: replace the first byte (int3) by the first byte of
> + * replacing opcode.
> + */
> + for (i = 0; i < nr_entries; i++)
> + text_poke(tp[i].addr, tp[i].opcode, sizeof(int3));
>
> 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.
> */
> + bp_patching.vec = NULL;
> + bp_patching.nr_entries = 0;
> + bp_patching.in_progress = false;
> +}
