> uprobes/x86: Don't WARN if an optimized probe can't be written TL;DR: Commit message only: it says all callers handle the error, but handler_chain() ignores a failed unoptimize on UPROBE_HANDLER_REMOVE, so with the WARN dropped that failure goes unreported.
> uprobes/x86: Don't WARN if an optimized probe can't be written > > __arch_uprobe_optimize() and set_orig_insn() do WARN_ON_ONCE() when > uprobe_write() fails, and user space can make it fail: > - -EINTR from get_user_pages_remote() when current is being killed. > - -ENOMEM when COW can't be broken, since memcg is at its limit. > - -EINVAL from __update_ref_ctr() when the process cleared the semaphore > of SDT probe. > > Kill the processes that call probed nop10. It warns in 8 of 8 runs: > > WARNING: arch/x86/kernel/uprobes.c:1186 at > arch_uprobe_optimize+0x291/0x3a0 > uprobe_notify_resume+0x1c3/0x4c0 > exit_to_user_mode_loop+0x63/0x4e0 > exc_int3+0x1f3/0x250 > > The next patch calls set_orig_insn() from fork(). Kill the processes that > fork and keep mmap_lock busy, while a trace event on nop10 is enabled and > disabled. The kernel with the next two patches and without this one warns > in 3 of 3 runs. The whole series in 0 of 3: > > WARNING: arch/x86/kernel/uprobes.c:1161 at set_orig_insn+0x161/0x170 > register_one_mm+0x154/0x180 > uprobe_dup_mmap+0x61/0xb0 > dup_mmap+0x149/0x9f0 > > The callers handle the error. arch_uprobe_optimize() marks uprobe as not > optimizable and byte 0 stays a breakpoint. set_orig_insn() returns > the error. Drop both WARNs. > > This depends on the previous patch. Without it WARN was the only sign of > the bytes that set_orig_insn() cannot restore. Hence no Fixes tag. > > Signed-off-by: Alexei Starovoitov <[email protected]> This isn't a bug, but the changelog says "The callers handle the error". Is that accurate for every caller of set_orig_insn()? set_orig_insn() is reached through remove_breakpoint(), and one of its callers drops the result: kernel/events/uprobes.c:handler_chain() { ... if (!filter_chain(uprobe, current->mm)) { WARN_ON(!uprobe_is_active(uprobe)); unapply_uprobe(uprobe, current->mm); } ... } unapply_uprobe() ORs the remove_breakpoint() result into its return value, but handler_chain() ignores it. So if a consumer returns UPROBE_HANDLER_REMOVE on an optimized nop10 and swbp_unoptimize() fails (-EINTR, -ENOMEM or -EINVAL as listed above), nothing is propagated or logged on this path once the WARN_ON_ONCE() is gone. The other path, register_for_each_vma(), does propagate the error. Could the changelog say that handler_chain() treats the removal as best-effort and ignores the result, instead of saying all callers handle the error? --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/39710454342
