linux-kernel - Re: [PATCH 2/2] uprobes: is_trap_at_addr: don't use get_user_pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4Bza94=39M7s1fNTZYFcx7QG1SotcdZ-O+WnyEza=rFkETA@mail.gmail.com>
Date: Wed, 10 Jul 2024 08:15:02 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Oleg Nesterov <oleg@...hat.com>
Cc: andrii@...nel.org, mhiramat@...nel.org, peterz@...radead.org, clm@...a.com, 
	jolsa@...nel.org, mingo@...nel.org, paulmck@...nel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] uprobes: is_trap_at_addr: don't use get_user_pages_remote()

On Wed, Jul 10, 2024 at 7:02 AM Oleg Nesterov <oleg@...hat.com> wrote:
>
> get_user_pages_remote() and the comment above it make no sense.
>
> There is no task_struct passed into get_user_pages_remote() anymore, and
> nowadays mm_account_fault() increments the current->min/maj_flt counters
> regardless of FAULT_FLAG_REMOTE.
>
> Reported-by: Andrii Nakryiko <andrii@...nel.org>
> Signed-off-by: Oleg Nesterov <oleg@...hat.com>
> ---
>  kernel/events/uprobes.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>

Makes sense

Acked-by: Andrii Nakryiko <andrii@...nel.org>

> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index d52b624a50fa..1bdf386485d4 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 read or write.
> + * Called with mm->mmap_lock held.
>   * Return 0 (success) or a negative errno.
>   */
>  int uprobe_write_opcode(struct arch_uprobe *auprobe, struct mm_struct *mm,
> @@ -2024,13 +2024,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
>         if (likely(result == 0))
>                 goto out;
>
> -       /*
> -        * The NULL 'tsk' here ensures that any faults that occur here
> -        * will not be accounted to the task.  'mm' *is* current->mm,
> -        * but we treat this as a 'remote' access since it is
> -        * essentially a kernel access to the memory.
> -        */
> -       result = get_user_pages_remote(mm, vaddr, 1, FOLL_FORCE, &page, NULL);
> +       result = get_user_pages(vaddr, 1, FOLL_FORCE, &page);
>         if (result < 0)
>                 return result;
>
> --
> 2.25.1.362.g51ebf55
>
>