Messages in this thread | | | Date | Thu, 22 Nov 2018 16:45:27 +0100 | From | Andrea Parri <> | Subject | Re: [Question] smp_wmb() in prepare_uprobe() |
| |
On Thu, Nov 22, 2018 at 04:05:24PM +0100, Oleg Nesterov wrote: > 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.
Thank you for the clarification; I'll send a patch with the fix shortly.
Andrea
> > Oleg. >
| |