On 07/08, Andrii Nakryiko wrote: > > On Sun, Jul 7, 2024 at 7:48 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote: > > > > And I forgot to mention... > > > > In any case __uprobe_unregister() can't ignore the error code from > > register_for_each_vma(). If it fails to restore the original insn, > > we should not remove this uprobe from uprobes_tree. > > > > Otherwise the next handle_swbp() will send SIGTRAP to the (no longer) > > probed application. > > Yep, that would be unfortunate (just like SIGILL sent when uretprobe > detects "improper" stack pointer progression, for example), In this case we a) assume that user-space tries to fool the kernel and b) the kernel can't handle this case in any case, thus uprobe_warn(). > but from > what I gather it's not really expected to fail on unregistration given > we successfully registered uprobe. Not really expected, and that is why the "TODO" comment in _unregister() was never implemented. Although the real reason is that we are lazy ;) But register_for_each_vma(NULL) can fail. Say, simply because kmalloc(GFP_KERNEL) in build_map_info() can fail even if it "never" should. A lot of other reasons. > I guess it's a decision between > leaking memory with an uprobe stuck in the tree or killing process due > to some very rare (or buggy) condition? Yes. I think in this case it is better to leak uprobe than kill the no longer probed task. Oleg.
