| From 09d3f015d1e1b4fee7e9bbdcf54201d239393391 Mon Sep 17 00:00:00 2001 |
| From: Andrea Parri <andrea.parri@amarulasolutions.com> |
| Date: Thu, 22 Nov 2018 17:10:31 +0100 |
| Subject: uprobes: Fix handle_swbp() vs. unregister() + register() race once more |
| |
| From: Andrea Parri <andrea.parri@amarulasolutions.com> |
| |
| commit 09d3f015d1e1b4fee7e9bbdcf54201d239393391 upstream. |
| |
| Commit: |
| |
| 142b18ddc8143 ("uprobes: Fix handle_swbp() vs unregister() + register() race") |
| |
| added the UPROBE_COPY_INSN flag, and corresponding smp_wmb() and smp_rmb() |
| memory barriers, to ensure that handle_swbp() uses fully-initialized |
| uprobes only. |
| |
| However, the smp_rmb() is mis-placed: this barrier should be placed |
| after handle_swbp() has tested for the flag, thus guaranteeing that |
| (program-order) subsequent loads from the uprobe can see the initial |
| stores performed by prepare_uprobe(). |
| |
| Move the smp_rmb() accordingly. Also amend the comments associated |
| to the two memory barriers to indicate their actual locations. |
| |
| Signed-off-by: Andrea Parri <andrea.parri@amarulasolutions.com> |
| Acked-by: Oleg Nesterov <oleg@redhat.com> |
| Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> |
| Cc: Andrew Morton <akpm@linux-foundation.org> |
| Cc: Arnaldo Carvalho de Melo <acme@redhat.com> |
| Cc: Jiri Olsa <jolsa@redhat.com> |
| Cc: Linus Torvalds <torvalds@linux-foundation.org> |
| Cc: Namhyung Kim <namhyung@kernel.org> |
| Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com> |
| Cc: Peter Zijlstra <peterz@infradead.org> |
| Cc: Stephane Eranian <eranian@google.com> |
| Cc: Thomas Gleixner <tglx@linutronix.de> |
| Cc: Vince Weaver <vincent.weaver@maine.edu> |
| Cc: stable@kernel.org |
| Fixes: 142b18ddc8143 ("uprobes: Fix handle_swbp() vs unregister() + register() race") |
| Link: http://lkml.kernel.org/r/20181122161031.15179-1-andrea.parri@amarulasolutions.com |
| Signed-off-by: Ingo Molnar <mingo@kernel.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| kernel/events/uprobes.c | 12 ++++++++++-- |
| 1 file changed, 10 insertions(+), 2 deletions(-) |
| |
| --- a/kernel/events/uprobes.c |
| +++ b/kernel/events/uprobes.c |
| @@ -612,7 +612,7 @@ static int prepare_uprobe(struct uprobe |
| BUG_ON((uprobe->offset & ~PAGE_MASK) + |
| UPROBE_SWBP_INSN_SIZE > PAGE_SIZE); |
| |
| - smp_wmb(); /* pairs with rmb() in find_active_uprobe() */ |
| + smp_wmb(); /* pairs with the smp_rmb() in handle_swbp() */ |
| set_bit(UPROBE_COPY_INSN, &uprobe->flags); |
| |
| out: |
| @@ -1860,10 +1860,18 @@ static void handle_swbp(struct pt_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 wmb() in install_breakpoint() */ |
| 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; |