Re: [PATCH 2/2] uprobes: is_trap_at_addr: don't use get_user_pages_remote()

[Andrii Nakryiko]

On Wed, Jul 10, 2024 at 7:02 AM Oleg Nesterov <oleg@xxxxxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
> ---
>  kernel/events/uprobes.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>

Makes sense

Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>

> 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
>
>