From: Oleg Nesterov <oleg@redhat.com>
To: Ingo Molnar <mingo@elte.hu>,
Peter Zijlstra <peterz@infradead.org>,
Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
Anton Arapov <anton@redhat.com>,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
linux-kernel@vger.kernel.org
Subject: [PATCH 7/7] uprobes: Fix the racy uprobe->flags manipulation
Date: Sun, 30 Sep 2012 21:42:27 +0200 [thread overview]
Message-ID: <20120930194227.GA11350@redhat.com> (raw)
In-Reply-To: <20120930194119.GA11278@redhat.com>
Multiple threads can manipulate uprobe->flags, this is obviously
unsafe. For example mmap can set UPROBE_COPY_INSN while register
tries to set UPROBE_RUN_HANDLER, the latter can also race with
can_skip_sstep() which clears UPROBE_SKIP_SSTEP.
Change this code to use bitops.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/events/uprobes.c | 28 ++++++++++++++--------------
1 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 8410388..3d8c815 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -79,11 +79,11 @@ static struct mutex uprobes_mmap_mutex[UPROBES_HASH_SZ];
static atomic_t uprobe_events = ATOMIC_INIT(0);
/* Have a copy of original instruction */
-#define UPROBE_COPY_INSN 0x1
+#define UPROBE_COPY_INSN 0
/* Dont run handlers when first register/ last unregister in progress*/
-#define UPROBE_RUN_HANDLER 0x2
+#define UPROBE_RUN_HANDLER 1
/* Can skip singlestep */
-#define UPROBE_SKIP_SSTEP 0x4
+#define UPROBE_SKIP_SSTEP 2
struct uprobe {
struct rb_node rb_node; /* node in the rb tree */
@@ -94,7 +94,7 @@ struct uprobe {
struct uprobe_consumer *consumers;
struct inode *inode; /* Also hold a ref to inode */
loff_t offset;
- int flags;
+ unsigned long flags;
struct arch_uprobe arch;
};
@@ -423,7 +423,7 @@ static struct uprobe *insert_uprobe(struct uprobe *uprobe)
spin_unlock(&uprobes_treelock);
/* For now assume that the instruction need not be single-stepped */
- uprobe->flags |= UPROBE_SKIP_SSTEP;
+ __set_bit(UPROBE_SKIP_SSTEP, &uprobe->flags);
return u;
}
@@ -466,7 +466,7 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs)
{
struct uprobe_consumer *uc;
- if (!(uprobe->flags & UPROBE_RUN_HANDLER))
+ if (!test_bit(UPROBE_RUN_HANDLER, &uprobe->flags))
return;
down_read(&uprobe->consumer_rwsem);
@@ -577,11 +577,11 @@ static int uprobe_copy_insn(struct uprobe *uprobe, struct file *file,
{
int ret = 0;
- if (uprobe->flags & UPROBE_COPY_INSN)
+ if (test_bit(UPROBE_COPY_INSN, &uprobe->flags))
return ret;
mutex_lock(&uprobe->copy_mutex);
- if (uprobe->flags & UPROBE_COPY_INSN)
+ if (test_bit(UPROBE_COPY_INSN, &uprobe->flags))
goto out;
ret = copy_insn(uprobe, file);
@@ -601,7 +601,7 @@ static int uprobe_copy_insn(struct uprobe *uprobe, struct file *file,
UPROBE_SWBP_INSN_SIZE > PAGE_SIZE);
smp_wmb(); /* pairs with rmb() in find_active_uprobe() */
- uprobe->flags |= UPROBE_COPY_INSN;
+ set_bit(UPROBE_COPY_INSN, &uprobe->flags);
ret = 0;
out:
mutex_unlock(&uprobe->copy_mutex);
@@ -852,7 +852,7 @@ int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer *
uprobe->consumers = NULL;
__uprobe_unregister(uprobe);
} else {
- uprobe->flags |= UPROBE_RUN_HANDLER;
+ set_bit(UPROBE_RUN_HANDLER, &uprobe->flags);
}
}
@@ -885,7 +885,7 @@ void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consume
if (consumer_del(uprobe, uc)) {
if (!uprobe->consumers) {
__uprobe_unregister(uprobe);
- uprobe->flags &= ~UPROBE_RUN_HANDLER;
+ clear_bit(UPROBE_RUN_HANDLER, &uprobe->flags);
}
}
@@ -1346,10 +1346,10 @@ bool uprobe_deny_signal(void)
*/
static bool can_skip_sstep(struct uprobe *uprobe, struct pt_regs *regs)
{
- if (uprobe->flags & UPROBE_SKIP_SSTEP) {
+ if (test_bit(UPROBE_SKIP_SSTEP, &uprobe->flags)) {
if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
return true;
- uprobe->flags &= ~UPROBE_SKIP_SSTEP;
+ clear_bit(UPROBE_SKIP_SSTEP, &uprobe->flags);
}
return false;
}
@@ -1428,7 +1428,7 @@ static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp)
* new and not-yet-analyzed uprobe at the same address, restart.
*/
smp_rmb(); /* pairs with wmb() in install_breakpoint() */
- if (uprobe && unlikely(!(uprobe->flags & UPROBE_COPY_INSN))) {
+ if (uprobe && unlikely(!test_bit(UPROBE_COPY_INSN, &uprobe->flags))) {
uprobe = NULL;
*is_swbp = 0;
}
--
1.5.5.1
next prev parent reply other threads:[~2012-09-30 19:41 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-30 19:41 [PATCH 0/7] uprobes: register/unregister bugfixes Oleg Nesterov
2012-09-30 19:41 ` [PATCH 1/7] uprobes/x86: Only rep+nop can be emulated correctly Oleg Nesterov
2012-10-06 7:20 ` Srikar Dronamraju
2012-09-30 19:42 ` [PATCH 2/7] uprobes: Don't return success if alloc_uprobe() fails Oleg Nesterov
2012-10-06 7:25 ` Srikar Dronamraju
2012-09-30 19:42 ` [PATCH 3/7] uprobes: Do not delete uprobe if uprobe_unregister() fails Oleg Nesterov
2012-10-06 8:48 ` Srikar Dronamraju
2012-09-30 19:42 ` [PATCH 4/7] uprobes: Fix handle_swbp() vs unregister() + register() race Oleg Nesterov
2012-10-02 18:42 ` Oleg Nesterov
2012-10-06 9:33 ` Srikar Dronamraju
2012-10-06 17:25 ` Oleg Nesterov
2012-10-06 17:37 ` Srikar Dronamraju
2012-10-06 18:53 ` Oleg Nesterov
2012-10-07 7:12 ` Srikar Dronamraju
2012-09-30 19:42 ` [PATCH 5/7] uprobes: Introduce uprobe_copy_insn() Oleg Nesterov
2012-10-06 9:45 ` Srikar Dronamraju
2012-10-06 17:10 ` Oleg Nesterov
2012-10-06 17:38 ` Srikar Dronamraju
2012-10-06 18:59 ` Oleg Nesterov
2012-10-07 7:14 ` Srikar Dronamraju
2012-09-30 19:42 ` [PATCH 6/7] uprobes: Fix uprobe_copy_insn() race with itself Oleg Nesterov
2012-10-06 9:52 ` Srikar Dronamraju
2012-09-30 19:42 ` Oleg Nesterov [this message]
2012-10-04 8:57 ` [PATCH 7/7] uprobes: Fix the racy uprobe->flags manipulation Anton Arapov
2012-10-06 9:54 ` Srikar Dronamraju
2012-09-30 19:44 ` [PATCH 0/7] uprobes: register/unregister bugfixes Oleg Nesterov
2012-10-01 12:55 ` Srikar Dronamraju
2012-10-01 14:03 ` Oleg Nesterov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120930194227.GA11350@redhat.com \
--to=oleg@redhat.com \
--cc=ananth@in.ibm.com \
--cc=anton@redhat.com \
--cc=bigeasy@linutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=srikar@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.