lkml.org 
[lkml]   [2018]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.

\
 
 \ /
  Last update: 2018-11-22 16:05    [W:0.056 / U:0.412 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site