All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: andrii@kernel.org, mhiramat@kernel.org, peterz@infradead.org
Cc: jolsa@kernel.org, rostedt@goodmis.org,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Subject: [PATCH v4 1/9] uprobes: document the usage of mm->mmap_lock
Date: Thu, 1 Aug 2024 15:27:09 +0200	[thread overview]
Message-ID: <20240801132709.GA8780@redhat.com> (raw)
In-Reply-To: <20240801132638.GA8759@redhat.com>

The comment above uprobe_write_opcode() is wrong, unapply_uprobe() calls
it under mmap_read_lock() and this is correct.

And it is completely unclear why register_for_each_vma() takes mmap_lock
for writing, add a comment to explain that mmap_write_lock() is needed to
avoid the following race:

	- A task T hits the bp installed by uprobe and calls
	  find_active_uprobe()

	- uprobe_unregister() removes this uprobe/bp

	- T calls find_uprobe() which returns NULL

	- another uprobe_register() installs the bp at the same address

	- T calls is_trap_at_addr() which returns true

	- T returns to handle_swbp() and gets SIGTRAP.

Reported-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/events/uprobes.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 73cc47708679..73dd12b09a7b 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -453,7 +453,7 @@ static int update_ref_ctr(struct uprobe *uprobe, struct mm_struct *mm,
  * @vaddr: the virtual address to store the opcode.
  * @opcode: opcode to be written at @vaddr.
  *
- * Called with mm->mmap_lock held for write.
+ * Called with mm->mmap_lock held for read or write.
  * Return 0 (success) or a negative errno.
  */
 int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm,
@@ -1046,7 +1046,13 @@ register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new)
 
 		if (err && is_register)
 			goto free;
-
+		/*
+		 * We take mmap_lock for writing to avoid the race with
+		 * find_active_uprobe() which takes mmap_lock for reading.
+		 * Thus this install_breakpoint() can not make
+		 * is_trap_at_addr() true right after find_uprobe()
+		 * returns NULL in find_active_uprobe().
+		 */
 		mmap_write_lock(mm);
 		vma = find_vma(mm, info->vaddr);
 		if (!vma || !valid_vma(vma, is_register) ||
-- 
2.25.1.362.g51ebf55


  reply	other threads:[~2024-08-01 13:27 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-01 13:26 [PATCH v4 0/9] uprobes: misc cleanups/simplifications Oleg Nesterov
2024-08-01 13:27 ` Oleg Nesterov [this message]
2024-08-05 11:56   ` [tip: perf/core] uprobes: document the usage of mm->mmap_lock tip-bot2 for Oleg Nesterov
2024-08-01 13:27 ` [PATCH v4 2/9] uprobes: is_trap_at_addr: don't use get_user_pages_remote() Oleg Nesterov
2024-08-05 11:56   ` [tip: perf/core] " tip-bot2 for Oleg Nesterov
2024-08-01 13:27 ` [PATCH v4 3/9] uprobes: simplify error handling for alloc_uprobe() Oleg Nesterov
2024-08-05 11:56   ` [tip: perf/core] " tip-bot2 for Andrii Nakryiko
2024-08-01 13:27 ` [PATCH v4 4/9] selftests/bpf: fix uprobe.path leak in bpf_testmod Oleg Nesterov
2024-08-05 11:56   ` [tip: perf/core] " tip-bot2 for Jiri Olsa
2024-08-01 13:27 ` [PATCH v4 5/9] uprobes: kill uprobe_register_refctr() Oleg Nesterov
2024-08-05 11:56   ` [tip: perf/core] " tip-bot2 for Oleg Nesterov
2024-08-01 13:27 ` [PATCH v4 6/9] uprobes: make uprobe_register() return struct uprobe * Oleg Nesterov
2024-08-05 11:56   ` [tip: perf/core] " tip-bot2 for Oleg Nesterov
2024-08-01 13:27 ` [PATCH v4 7/9] uprobes: change uprobe_register() to use uprobe_unregister() instead of __uprobe_unregister() Oleg Nesterov
2024-08-05 11:56   ` [tip: perf/core] " tip-bot2 for Oleg Nesterov
2024-08-01 13:27 ` [PATCH v4 8/9] uprobes: fold __uprobe_unregister() into uprobe_unregister() Oleg Nesterov
2024-08-05 11:56   ` [tip: perf/core] " tip-bot2 for Oleg Nesterov
2024-08-01 13:27 ` [PATCH v4 9/9] uprobes: shift put_uprobe() from delete_uprobe() to uprobe_unregister() Oleg Nesterov
2024-08-05 11:56   ` [tip: perf/core] " tip-bot2 for Oleg Nesterov
2024-08-01 13:36 ` [PATCH v4 0/9] uprobes: misc cleanups/simplifications Peter Zijlstra
2024-08-01 18:58   ` Andrii Nakryiko
2024-08-01 21:13     ` Andrii Nakryiko
2024-08-02  8:27       ` Peter Zijlstra
2024-08-02  9:25       ` Peter Zijlstra
2024-08-02 11:02         ` Adrian Hunter
2024-08-02 17:14           ` Adrian Hunter

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=20240801132709.GA8780@redhat.com \
    --to=oleg@redhat.com \
    --cc=andrii@kernel.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    /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.