All of lore.kernel.org
 help / color / mirror / Atom feed
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 3/7] uprobes: Do not delete uprobe if uprobe_unregister() fails
Date: Sun, 30 Sep 2012 21:42:04 +0200	[thread overview]
Message-ID: <20120930194204.GA11326@redhat.com> (raw)
In-Reply-To: <20120930194119.GA11278@redhat.com>

delete_uprobe() must not be called if register_for_each_vma(false)
fails to remove all breakpoints, __uprobe_unregister() is correct.
The problem is that register_for_each_vma(false) always returns 0
and thus this logic does not work.

1. Change verify_opcode() to return 0 rather than -EINVAL when
   unregister detects the !is_swbp insn, we can treat this case
   as success and currently unregister paths ignore the error
   code anyway.

2. Change remove_breakpoint() to propagate the error code from
   write_opcode().

3. Change register_for_each_vma(is_register => false) to remove
   as much breakpoints as possible but return non-zero if
   remove_breakpoint() fails at least once.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/events/uprobes.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 3ec114c..6058231 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -203,7 +203,7 @@ static int verify_opcode(struct page *page, unsigned long vaddr, uprobe_opcode_t
 			return 0;
 	} else {
 		if (!is_swbp)		/* unregister: was it changed by us? */
-			return -EINVAL;
+			return 0;
 	}
 
 	return 1;
@@ -616,15 +616,15 @@ install_breakpoint(struct uprobe *uprobe, struct mm_struct *mm,
 	return ret;
 }
 
-static void
+static int
 remove_breakpoint(struct uprobe *uprobe, struct mm_struct *mm, unsigned long vaddr)
 {
 	/* can happen if uprobe_register() fails */
 	if (!test_bit(MMF_HAS_UPROBES, &mm->flags))
-		return;
+		return 0;
 
 	set_bit(MMF_RECALC_UPROBES, &mm->flags);
-	set_orig_insn(&uprobe->arch, mm, vaddr);
+	return set_orig_insn(&uprobe->arch, mm, vaddr);
 }
 
 /*
@@ -740,7 +740,7 @@ static int register_for_each_vma(struct uprobe *uprobe, bool is_register)
 		struct mm_struct *mm = info->mm;
 		struct vm_area_struct *vma;
 
-		if (err)
+		if (err && is_register)
 			goto free;
 
 		down_write(&mm->mmap_sem);
@@ -756,7 +756,7 @@ static int register_for_each_vma(struct uprobe *uprobe, bool is_register)
 		if (is_register)
 			err = install_breakpoint(uprobe, mm, vma, info->vaddr);
 		else
-			remove_breakpoint(uprobe, mm, info->vaddr);
+			err |= remove_breakpoint(uprobe, mm, info->vaddr);
 
  unlock:
 		up_write(&mm->mmap_sem);
-- 
1.5.5.1


  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 ` Oleg Nesterov [this message]
2012-10-06  8:48   ` [PATCH 3/7] uprobes: Do not delete uprobe if uprobe_unregister() fails 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 ` [PATCH 7/7] uprobes: Fix the racy uprobe->flags manipulation Oleg Nesterov
2012-10-04  8:57   ` 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=20120930194204.GA11326@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.