From: Jiri Olsa jolsa@kernel.org
[ Upstream commit 4363264111e1297fa37aa39b0598faa19298ecca ]
If uprobe handler changes instruction pointer we still execute single step) or emulate the original instruction and increment the (new) ip with its length.
This makes the new instruction pointer bogus and application will likely crash on illegal instruction execution.
If user decided to take execution elsewhere, it makes little sense to execute the original instruction, so let's skip it.
Acked-by: Oleg Nesterov oleg@redhat.com Acked-by: Andrii Nakryiko andrii@kernel.org Signed-off-by: Jiri Olsa jolsa@kernel.org Link: https://lore.kernel.org/r/20250916215301.664963-3-jolsa@kernel.org Signed-off-by: Alexei Starovoitov ast@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
Summary - This is a small, well-scoped bug fix in the generic uprobes core that prevents executing or emulating the original instruction when a uprobe consumer handler has redirected the instruction pointer (IP). The previous behavior could corrupt the new IP and crash the traced application. The change is minimal (7 lines), does not add features, and aligns with expected semantics. It is suitable for stable backport.
What changed - In `handle_swbp()`, after running consumer handlers, the patch adds an early exit if the handler changed IP away from the breakpoint address: - New check added: `kernel/events/uprobes.c:2772` - Surrounding context: - Handler invocation: `kernel/events/uprobes.c:2769` - Emulation/single-step path: `kernel/events/uprobes.c:2778` (arch emulation) and `kernel/events/uprobes.c:2781` (XOL single-step prep). - The key addition is: - `kernel/events/uprobes.c:2772`: `if (instruction_pointer(regs) != bp_vaddr) goto out;`
Why the bug happens - Before this change, `handle_swbp()` always proceeded to emulate (`arch_uprobe_skip_sstep`) or to prepare out-of-line single-step (`pre_ssout`) of the original instruction even if the handler altered IP. On x86 and other arches, instruction emulation/step advances IP by the probed instruction’s length; doing that after a handler-set new IP advances the wrong address, making the IP bogus and often leading to SIGILL. - Where emulation executes: `kernel/events/uprobes.c:2778` - Where XOL single-step is prepared: `kernel/events/uprobes.c:2781` - The patch fixes this by skipping the emulate/sstep path if IP was changed by the handler, which is the correct intent when a handler redirects control flow.
Evidence in current/mainline and in stable - This exact fix is present in mainline commit 4363264111e12 (“uprobe: Do not emulate/sstep original instruction when ip is changed”) and adds only the early-out check in `handle_swbp()` (see `kernel/events/uprobes.c:2769`–`2785` in the current tree). - Affected stable trees (e.g., 6.1/6.6/6.10/6.17) lack this check and will incorrectly emulate/step even after IP changes. In your 6.17 workspace, `handle_swbp()` calls `handler_chain()` and then proceeds directly to emulation/step without guarding against an IP change: - Handler call: `kernel/events/uprobes.c:2742` - Emulation call: `kernel/events/uprobes.c:2744` - Single-step prep: `kernel/events/uprobes.c:2747`
Risk and side effects - Scope: Single function (`handle_swbp()`), 7 insertions, no API or architectural change. - Behavior change: Only when a handler changes IP; in that case, we skip executing the original instruction. This matches handler intent and prevents crashes. - Concurrency/locking: The check reads `instruction_pointer(regs)` and compares to `bp_vaddr` under the same conditions as the rest of the function; no new locking or ordering requirements. - Cross-arch impact: Safe and correct. All arches’ `arch_uprobe_skip_sstep()` implementations emulate or adjust IP assuming execution should continue at the original site; skipping this when IP was redirected avoids incorrect behavior. - No dependency on unrelated features (e.g., the `arch_uprobe_optimize()` call that exists in some newer trees is not part of this change and isn’t required for correctness).
Stable tree criteria - Fixes a user-visible crash-causing bug in uprobes (tracing/instrumentation). - Minimal, contained change with clear intent and low regression risk. - No new features or ABI changes. - Acked by maintainers and merged into mainline.
Conclusion - This is a clear, low-risk bug fix preventing incorrect emulation/single-step after handlers redirect IP. It should be backported to stable kernels.
kernel/events/uprobes.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 7ca1940607bd8..2b32c32bcb776 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -2741,6 +2741,13 @@ static void handle_swbp(struct pt_regs *regs)
handler_chain(uprobe, regs);
+ /* + * If user decided to take execution elsewhere, it makes little sense + * to execute the original instruction, so let's skip it. + */ + if (instruction_pointer(regs) != bp_vaddr) + goto out; + if (arch_uprobe_skip_sstep(&uprobe->arch, regs)) goto out;