Messages in this thread | | | Date | Fri, 25 Nov 2022 22:37:24 +0900 | From | Masami Hiramatsu (Google) <> | Subject | Re: [PATCH -tip] x86/kprobes: Handle removed INT3 in do_int3() |
| |
On Fri, 25 Nov 2022 08:41:19 +0100 Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Nov 25, 2022 at 10:09:02AM +0900, Masami Hiramatsu (Google) wrote: > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > Since x86 doesn't use stop_machine() to patch the kernel text, > > there is a small chance that the another CPU removes the INT3 > > during do_int3(). In this case, if no INT3 notifier callbacks > > handled that, the kernel calls die() because of a stray INT3. > > Please clarify; how would that happen? Should not everybody modifying > text take text_mutex ?
The text_mutex doesn't stop executing do_int3() since do_int3() is an exception and must not be blocked. That mutex is only blocking the other kernel text modifiers, not INT3 handling.
If there is only kprobe using INT3, this must not happen because kprobe_int3_handler() always find a struct kprobe corresponding to the INT3 address. (from this reason, the current code is wrong too)
However, if there are other INT3 callbacks (e.g. alternatives and ftrace via text_poke_bp*()) managing the INT3, this can happen. The possible scenario is here;
1. CPU0 hits an INT3 which is managed by text_poke_bp().
2. CPU1 removes the INT3 from the text and *start* calling text_poke_sync(). (note that text_poke_sync() will sync core to serialize pipeline, thus the memory and dcache already updated)
3. CPU0 calls kprobe_int3_handler(), but it can not find the corresponding kprobe from the address. Thus it reads the instruction at the address, and find it is not INT3 anymore.
4. CPU0's kprobe_int3_handler() sets the address to the regs->ip, and returns 1, which means "this INT3 is handled".
5. CPU0 returns from do_int3() and exit the exception, then it handles the do_sync_core() via IPI.
6. CPU1 finish the text_poke_sync().
This works fine, but it is not handled by the INT3 owner's callback.
Oh! maybe we don't need to handle remove INT3 case, because all callback MUST handle its own INT3. This can be done simply using text_poke_sync() because it use an IPI, and the IPI is not handled until all running INT3 handlers exit.
OK, let me update the patch to just drop the removed INT3 handling from kprobes.
Thank you,
-- Masami Hiramatsu (Google) <mhiramat@kernel.org>
| |