linux-kernel - Re: [PATCH 40/41] x86/alternatives: Rename 'text_poke_sync()' to 'text_poke_sync_each

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <DA9DB9BF-2179-4C53-86A8-6205346AA76F@zytor.com>
Date: Tue, 01 Apr 2025 21:10:55 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org
CC: Juergen Gross <jgross@...e.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>, Borislav Petkov <bp@...en8.de>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 40/41] x86/alternatives: Rename 'text_poke_sync()' to 'text_poke_sync_each_cpu()'

On March 27, 2025 1:53:53 PM PDT, Ingo Molnar <mingo@...nel.org> wrote:
>Unlike sync_core(), text_poke_sync() is a very heavy operation, as
>it sends an IPI to every online CPU in the system and waits for
>completion.
>
>Reflect this in the name.
>
>Signed-off-by: Ingo Molnar <mingo@...nel.org>
>---
> arch/x86/include/asm/text-patching.h |  2 +-
> arch/x86/kernel/alternative.c        | 12 ++++++------
> arch/x86/kernel/kprobes/core.c       |  4 ++--
> arch/x86/kernel/kprobes/opt.c        |  4 ++--
> arch/x86/kernel/module.c             |  2 +-
> 5 files changed, 12 insertions(+), 12 deletions(-)
>
>diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
>index 611957617278..ff30aa1d0c47 100644
>--- a/arch/x86/include/asm/text-patching.h
>+++ b/arch/x86/include/asm/text-patching.h
>@@ -32,7 +32,7 @@ extern void apply_relocation(u8 *buf, const u8 * const instr, size_t instrlen, u
>  * an inconsistent instruction while you patch.
>  */
> extern void *text_poke(void *addr, const void *opcode, size_t len);
>-extern void text_poke_sync(void);
>+extern void text_poke_sync_each_cpu(void);
> extern void *text_poke_kgdb(void *addr, const void *opcode, size_t len);
> extern void *text_poke_copy(void *addr, const void *opcode, size_t len);
> #define text_poke_copy text_poke_copy
>diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
>index 1b523496a2f6..32d3707d7963 100644
>--- a/arch/x86/kernel/alternative.c
>+++ b/arch/x86/kernel/alternative.c
>@@ -2445,7 +2445,7 @@ static void do_sync_core(void *info)
> 	sync_core();
> }
> 
>-void text_poke_sync(void)
>+void text_poke_sync_each_cpu(void)
> {
> 	on_each_cpu(do_sync_core, NULL, 1);
> }
>@@ -2469,8 +2469,8 @@ struct text_poke_int3_loc {
> #define TP_ARRAY_NR_ENTRIES_MAX (PAGE_SIZE / sizeof(struct text_poke_int3_loc))
> 
> static struct text_poke_int3_array {
>-	int nr_entries;
> 	struct text_poke_int3_loc vec[TP_ARRAY_NR_ENTRIES_MAX];
>+	int nr_entries;
> } tp_array;
> 
> static DEFINE_PER_CPU(atomic_t, tp_array_refs);
>@@ -2649,7 +2649,7 @@ static void text_poke_int3_batch_process(void)
> 		text_poke(text_poke_int3_addr(&tp_array.vec[i]), &int3, INT3_INSN_SIZE);
> 	}
> 
>-	text_poke_sync();
>+	text_poke_sync_each_cpu();
> 
> 	/*
> 	 * Second step: update all but the first byte of the patched range.
>@@ -2711,7 +2711,7 @@ static void text_poke_int3_batch_process(void)
> 		 * not necessary and we'd be safe even without it. But
> 		 * better safe than sorry (plus there's not only Intel).
> 		 */
>-		text_poke_sync();
>+		text_poke_sync_each_cpu();
> 	}
> 
> 	/*
>@@ -2732,13 +2732,13 @@ static void text_poke_int3_batch_process(void)
> 	}
> 
> 	if (do_sync)
>-		text_poke_sync();
>+		text_poke_sync_each_cpu();
> 
> 	/*
> 	 * Remove and wait for refs to be zero.
> 	 *
> 	 * Notably, if after step-3 above the INT3 got removed, then the
>-	 * text_poke_sync() will have serialized against any running INT3
>+	 * text_poke_sync_each_cpu() will have serialized against any running INT3
> 	 * handlers and the below spin-wait will not happen.
> 	 *
> 	 * IOW. unless the replacement instruction is INT3, this case goes
>diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
>index 09608fd93687..5e35c95524dc 100644
>--- a/arch/x86/kernel/kprobes/core.c
>+++ b/arch/x86/kernel/kprobes/core.c
>@@ -808,7 +808,7 @@ void arch_arm_kprobe(struct kprobe *p)
> 	u8 int3 = INT3_INSN_OPCODE;
> 
> 	text_poke(p->addr, &int3, 1);
>-	text_poke_sync();
>+	text_poke_sync_each_cpu();
> 	perf_event_text_poke(p->addr, &p->opcode, 1, &int3, 1);
> }
> 
>@@ -818,7 +818,7 @@ void arch_disarm_kprobe(struct kprobe *p)
> 
> 	perf_event_text_poke(p->addr, &int3, 1, &p->opcode, 1);
> 	text_poke(p->addr, &p->opcode, 1);
>-	text_poke_sync();
>+	text_poke_sync_each_cpu();
> }
> 
> void arch_remove_kprobe(struct kprobe *p)
>diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
>index 54bc5e7c6886..5efa7b50bbb3 100644
>--- a/arch/x86/kernel/kprobes/opt.c
>+++ b/arch/x86/kernel/kprobes/opt.c
>@@ -513,11 +513,11 @@ void arch_unoptimize_kprobe(struct optimized_kprobe *op)
> 	       JMP32_INSN_SIZE - INT3_INSN_SIZE);
> 
> 	text_poke(addr, new, INT3_INSN_SIZE);
>-	text_poke_sync();
>+	text_poke_sync_each_cpu();
> 	text_poke(addr + INT3_INSN_SIZE,
> 		  new + INT3_INSN_SIZE,
> 		  JMP32_INSN_SIZE - INT3_INSN_SIZE);
>-	text_poke_sync();
>+	text_poke_sync_each_cpu();
> 
> 	perf_event_text_poke(op->kp.addr, old, JMP32_INSN_SIZE, new, JMP32_INSN_SIZE);
> }
>diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
>index a7998f351701..1c598c90e24d 100644
>--- a/arch/x86/kernel/module.c
>+++ b/arch/x86/kernel/module.c
>@@ -206,7 +206,7 @@ static int write_relocate_add(Elf64_Shdr *sechdrs,
> 				   write, apply);
> 
> 	if (!early) {
>-		text_poke_sync();
>+		text_poke_sync_each_cpu();
> 		mutex_unlock(&text_mutex);
> 	}
> 

Is that the only use case we have for syncing all CPUs?