On 09/18, Adriano Cordova wrote: > > find_uprobe_rcu() backs handle_swbp() and handle_syscall_uprobe(). Make > it skip a uprobe whose instruction has not been analyzed yet, so neither > path can use an uninitialized ->arch. An unprepared uprobe has no > breakpoint installed (install_breakpoint() prepares before set_swbp()), > so the trap path treats it as not registered yet. This also gives > handle_syscall_uprobe() the check it was missing.
Sorry, I am confused. Can you explain in more details what problem(s) this series tries to fix? handle_swbp() checks UPROBE_COPY_INSN. handle_syscall_uprobe() is only called by sys_uprobe() which checks in_uprobe_trampoline(). So it can only be called after the probed insn has been analyzed and optimized. Oleg. > Signed-off-by: Adriano Cordova <[email protected]> > --- > kernel/events/uprobes.c | 35 +++++++++++++++-------------------- > 1 file changed, 15 insertions(+), 20 deletions(-) > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index 686585539ebf..bdc05fdce0a0 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -924,8 +924,20 @@ static struct uprobe *find_uprobe_rcu(struct inode > *inode, loff_t offset) > * try again as we might have missed the element (false > * negative). If seqcount is unchanged, search truly failed. > */ > - if (node) > - return __node_2_uprobe(node); > + if (node) { > + struct uprobe *uprobe = __node_2_uprobe(node); > + > + /* > + * A uprobe is inserted before it is prepared; do > + * not hand it out until ->arch is usable. > + */ > + if (!test_bit(UPROBE_COPY_INSN, &uprobe->flags)) > + return NULL; > + > + /* Pairs with the smp_wmb() in prepare_uprobe(). */ > + smp_rmb(); > + return uprobe; > + } > } while (read_seqcount_retry(&uprobes_seqcount, seq)); > > return NULL; > @@ -1123,7 +1135,7 @@ static int prepare_uprobe(struct uprobe *uprobe, struct > file *file, > if (ret) > goto out; > > - smp_wmb(); /* pairs with the smp_rmb() in handle_swbp() */ > + smp_wmb(); /* pairs with the smp_rmb() in find_uprobe_rcu() */ > set_bit(UPROBE_COPY_INSN, &uprobe->flags); > > out: > @@ -2743,23 +2755,6 @@ static void handle_swbp(struct pt_regs *regs) > /* change it in advance for ->handler() and restart */ > instruction_pointer_set(regs, bp_vaddr); > > - /* > - * TODO: move copy_insn/etc into _register and remove this hack. > - * After we hit the bp, _unregister + _register can install the > - * new and not-yet-analyzed uprobe at the same address, restart. > - */ > - if (unlikely(!test_bit(UPROBE_COPY_INSN, &uprobe->flags))) > - goto out; > - > - /* > - * Pairs with the smp_wmb() in prepare_uprobe(). > - * > - * Guarantees that if we see the UPROBE_COPY_INSN bit set, then > - * we must also see the stores to &uprobe->arch performed by the > - * prepare_uprobe() call. > - */ > - smp_rmb(); > - > /* Tracing handlers use ->utask to communicate with fetch methods */ > if (!get_utask()) > goto out; > -- > 2.51.0 >
