uprobes: Introduce find_active_uprobe() helper
authorOleg Nesterov <oleg@redhat.com>
Tue, 29 May 2012 19:28:57 +0000 (21:28 +0200)
committerIngo Molnar <mingo@kernel.org>
Wed, 6 Jun 2012 15:15:17 +0000 (17:15 +0200)
No functional changes. Move the "find uprobe" code from
handle_swbp() to the new helper, find_active_uprobe().

Note: with or without this change, the find-active-uprobe logic
is not exactly right. We can race with another thread which
unmaps the memory with the valid uprobe before we take
mm->mmap_sem. We can't find this uprobe simply because
find_vma() fails. In this case we wrongly assume that this trap
was not caused by uprobe and send the erroneous SIGTRAP. See the
next changes.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Anton Arapov <anton@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20120529192857.GC8057@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
kernel/events/uprobes.c

index a0dbc87a2ec6786884fac1981ca83637e23ab012..eaf4d55fd424eed37a4a4a3d6871fbbcaeb09b22 100644 (file)
@@ -1489,38 +1489,47 @@ static bool can_skip_sstep(struct uprobe *uprobe, struct pt_regs *regs)
        return false;
 }
 
-/*
- * Run handler and ask thread to singlestep.
- * Ensure all non-fatal signals cannot interrupt thread while it singlesteps.
- */
-static void handle_swbp(struct pt_regs *regs)
+static struct uprobe *find_active_uprobe(unsigned long bp_vaddr)
 {
+       struct mm_struct *mm = current->mm;
+       struct uprobe *uprobe = NULL;
        struct vm_area_struct *vma;
-       struct uprobe_task *utask;
-       struct uprobe *uprobe;
-       struct mm_struct *mm;
-       unsigned long bp_vaddr;
 
-       uprobe = NULL;
-       bp_vaddr = uprobe_get_swbp_addr(regs);
-       mm = current->mm;
        down_read(&mm->mmap_sem);
        vma = find_vma(mm, bp_vaddr);
 
-       if (vma && vma->vm_start <= bp_vaddr && valid_vma(vma, false)) {
-               struct inode *inode;
-               loff_t offset;
+       if (vma && vma->vm_start <= bp_vaddr) {
+               if (valid_vma(vma, false)) {
+                       struct inode *inode;
+                       loff_t offset;
 
-               inode = vma->vm_file->f_mapping->host;
-               offset = bp_vaddr - vma->vm_start;
-               offset += (vma->vm_pgoff << PAGE_SHIFT);
-               uprobe = find_uprobe(inode, offset);
+                       inode = vma->vm_file->f_mapping->host;
+                       offset = bp_vaddr - vma->vm_start;
+                       offset += (vma->vm_pgoff << PAGE_SHIFT);
+                       uprobe = find_uprobe(inode, offset);
+               }
        }
 
        srcu_read_unlock_raw(&uprobes_srcu, current->uprobe_srcu_id);
        current->uprobe_srcu_id = -1;
        up_read(&mm->mmap_sem);
 
+       return uprobe;
+}
+
+/*
+ * Run handler and ask thread to singlestep.
+ * Ensure all non-fatal signals cannot interrupt thread while it singlesteps.
+ */
+static void handle_swbp(struct pt_regs *regs)
+{
+       struct uprobe_task *utask;
+       struct uprobe *uprobe;
+       unsigned long bp_vaddr;
+
+       bp_vaddr = uprobe_get_swbp_addr(regs);
+       uprobe = find_active_uprobe(bp_vaddr);
+
        if (!uprobe) {
                /* No matching uprobe; signal SIGTRAP. */
                send_sig(SIGTRAP, current, 0);