On Tue, 17 Jun 2025 19:41:59 +0900 Masami Hiramatsu (Google) mhiramat@kernel.org wrote:
Eventually, I found a bug in text_poke, and jump_label (tracepoint) hit the bug.
The jump_label uses 2 different APIs (single and batch) which independently takes text_mutex lock.
smp_text_poke_single() __jump_label_transform() jump_label_transform() --> lock text_mutex
smp_text_poke_batch_add() arch_jump_label_transform_queue() -> lock text_mutex
smp_text_poke_batch_finish() arch_jump_label_transform_apply() -> lock text_mutex
This is allowed by commit 8a6a1b4e0ef1 ("x86/alternatives: Remove the mixed-patching restriction on smp_text_poke_single()"), but smp_text_poke_single() still expects that the batched APIs are run in the same text_mutex lock region. Thus if user calls those APIs in the below order;
arch_jump_label_transform_queue(addr1) jump_label_transform(addr2) arch_jump_label_transform_apply()
And if the addr1 > addr2, the bsearch on the array does not work, and failed to handle int3!
Nice catch!
This can explain the disappeared int3 case. If it happens right before int3 is overwritten, that int3 will be overwritten when the int3 handler dumps the code, but text_poke_array_refs is still 1.
It seems that commit c8976ade0c1b ("x86/alternatives: Simplify smp_text_poke_single() by using tp_vec and existing APIs") introduced this problem, because it shares the global array in the text_poke_batch and text_poke_single. Before that commit, text_poke_single (text_poke_bp) uses its local variable.
To fix this issue, Use smp_text_poke_batch_add() in smp_text_poke_single(), which checks whether the array sorted and the array index does not overflow.
Please test below;
From e2a49c7cefb4148ea3142c752396d39f103c9f4d Mon Sep 17 00:00:00 2001
From: "Masami Hiramatsu (Google)" mhiramat@kernel.org Date: Tue, 17 Jun 2025 19:18:37 +0900 Subject: [PATCH] x86: alternative: Fix int3 handling failure from broken text_poke array
Since smp_text_poke_single() does not expect there is another text_poke request is queued, it can make text_poke_array not sorted or cause a buffer overflow on the text_poke_array.vec[]. This will cause an Oops in int3, or kernel page fault if it causes a buffer overflow.
I would add more of what you found above in the change log. And the issue that was triggered I don't think was because of a buffer overflow. It was because an entry was added to the text_poke_array out of order causing the bsearch to fail.
Please add to the change log that the issue is that smp_text_poke_single() can be called while smp_text_poke_batch*() is being used. The locking is around the called functions but nothing prevents them from being intermingled.
This means that if we have:
CPU 0 CPU 1 CPU 2 ----- ----- -----
smp_text_poke_batch_add()
smp_text_poke_single() <<-- Adds out of order
<int3> [Fails o find address in text_poke_array ] OOPS!
No overflow. This could possibly happen with just two entries!
Use smp_text_poke_batch_add() instead of __smp_text_poke_batch_add() so that it correctly flush the queue if needed.
Reported-by: Linux Kernel Functional Testing lkft@linaro.org Closes: https://lore.kernel.org/all/CA+G9fYsLu0roY3DV=tKyqP7FEKbOEETRvTDhnpPxJGbA=Cg... Fixes: 8976ade0c1b ("x86/alternatives: Simplify smp_text_poke_single() by using tp_vec and existing APIs") Signed-off-by: Masami Hiramatsu (Google)
Reviewed-by: Steven Rostedt (Google) rostedt@goodmis.org
-- Steve
mhiramat@kernel.org --- arch/x86/kernel/alternative.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index ecfe7b497cad..8038951650c6 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -3107,6 +3107,6 @@ void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, c */ void __ref smp_text_poke_single(void *addr, const void *opcode, size_t len, const void *emulate) {
- __smp_text_poke_batch_add(addr, opcode, len, emulate);
- smp_text_poke_batch_add(addr, opcode, len, emulate); smp_text_poke_batch_finish();
}