Messages in this thread | | | Date | Thu, 22 Nov 2018 16:05:24 +0100 | From | Oleg Nesterov <> | Subject | Re: [Question] smp_wmb() in prepare_uprobe() |
| |
On 11/22, Andrea Parri wrote: > > > See 142b18ddc81439acda4bc4231b291e99fe67d507 ("uprobes: Fix handle_swbp() > > vs unregister() + register() race") and the comment above this rmb(). > > Mmh..., at first glance, this suggests me that the above set_bit() and > test_bit() to/from uprobe->flags are among these memory accesses. But > this doesn't make sense to me: these accesses do not "alternate" (i.e., > they both appear after the corresponding barrier..); instead I'd expect > something like (on top of the above diff): > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index 2d29977522017..a75b9a08dee54 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -2178,10 +2178,18 @@ static void handle_swbp(struct pt_regs *regs) > * After we hit the bp, _unregister + _register can install the > * new and not-yet-analyzed uprobe at the same address, restart. > */ > - smp_rmb(); /* pairs with the smp_wmb() in prepare_uprobe() */ > 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 (can) also see the stores to &uprobe->arch performed > + * by prepare_uprobe() (say). > + */ > + smp_rmb();
OOPS, you are right! Thanks.
Oleg.
| |