Re: [PATCH v2 02/12] uprobes: correct mmap_sem locking assumptions in uprobe_write_opcode()

Masami Hiramatsu
2024-07-03T13:15:29Z

On Mon, 1 Jul 2024 15:39:25 -0700
Andrii Nakryiko <andrii@kernel.org> wrote:

> It seems like uprobe_write_opcode() doesn't require writer locked
> mmap_sem, any lock (reader or writer) should be sufficient. This was
> established in a discussion in [0] and looking through existing code
> seems to confirm that there is no need for write-locked mmap_sem.
>
> Fix the comment to state this clearly.
>
> [0] https://lore.kernel.org/linux-trace-kernel/20240625190748.GC14254@redhat.com/
>
> Fixes: 29dedee0e693 ("uprobes: Add mem_cgroup_charge_anon() into uprobe_write_opcode()")

nit: why this has Fixes but [01/12] doesn't?

Should I pick both to fixes branch?

Thank you,

> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> kernel/events/uprobes.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 081821fd529a..f87049c08ee9 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,
> --
> 2.43.0
>

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>