Re: [PATCH RFC v4 01/12] x86/mm: add untagged_addr_remote_unlocked()

[Suren Baghdasaryan]

On Fri, Jul 24, 2026 at 3:32 PM Rik van Riel <riel@xxxxxxxxxxx> wrote:
>
> __access_remote_vm() reads another process's memory under the mmap lock. On
> large machines running big multi-threaded applications, that lock is
> contended between readers and writers: an mmap() or munmap() in one thread
> stalls readers like /proc/PID/cmdline, /proc/PID/environ, or
> process_vm_readv(), even though the memory they read is almost always
> resident and reachable under the per-VMA lock instead.
>
> Looking up the VMA first requires untagging the address.
> untagged_addr_remote() asserts the mmap lock only because it reads
> mm->context.untag_mask, which can race with the write in mm_enable_lam().
>
> That mask changes only while the target is single-threaded, when it
> enables LAM or resets it at exec. The read need not be stable: it is
> already read locklessly elsewhere, and a remote untag is best-effort.
>
> Add untagged_addr_remote_unlocked() for callers that have not taken the
> mmap lock, and annotate access to mm->context.untag_mask with READ_ONCE()
> and WRITE_ONCE() so the existing lockless reads are explicit and
> KCSAN-clean. untagged_addr_remote() keeps its assertion and shares the same
> code.
>
> Assisted-by: Claude:claude-opus-4.8
> Acked-by: Usama Arif <usama.arif@xxxxxxxxx>
> Signed-off-by: Rik van Riel <riel@xxxxxxxxxxx>

Reviewed-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>

> ---
>  arch/x86/include/asm/mmu_context.h |  6 +++---
>  arch/x86/include/asm/uaccess_64.h  | 15 ++++++++++++---
>  arch/x86/kernel/process_64.c       |  4 ++--
>  include/linux/uaccess.h            |  7 +++++++
>  4 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
> index ef5b507de34e..cee710f64658 100644
> --- a/arch/x86/include/asm/mmu_context.h
> +++ b/arch/x86/include/asm/mmu_context.h
> @@ -100,18 +100,18 @@ static inline unsigned long mm_lam_cr3_mask(struct mm_struct *mm)
>  static inline void dup_lam(struct mm_struct *oldmm, struct mm_struct *mm)
>  {
>         mm->context.lam_cr3_mask = oldmm->context.lam_cr3_mask;
> -       mm->context.untag_mask = oldmm->context.untag_mask;
> +       WRITE_ONCE(mm->context.untag_mask, READ_ONCE(oldmm->context.untag_mask));
>  }
>
>  #define mm_untag_mask mm_untag_mask
>  static inline unsigned long mm_untag_mask(struct mm_struct *mm)
>  {
> -       return mm->context.untag_mask;
> +       return READ_ONCE(mm->context.untag_mask);
>  }
>
>  static inline void mm_reset_untag_mask(struct mm_struct *mm)
>  {
> -       mm->context.untag_mask = -1UL;
> +       WRITE_ONCE(mm->context.untag_mask, -1UL);
>  }
>
>  #define arch_pgtable_dma_compat arch_pgtable_dma_compat
> diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
> index 20de34cc9aa6..a095165eb648 100644
> --- a/arch/x86/include/asm/uaccess_64.h
> +++ b/arch/x86/include/asm/uaccess_64.h
> @@ -39,18 +39,27 @@ static inline unsigned long __untagged_addr(unsigned long addr)
>         (__force __typeof__(addr))__untagged_addr(__addr);              \
>  })
>
> +/*
> + * mm->context.untag_mask changes only when the target enables LAM or execs,
> + * always single-threaded. An unlocked remote reader can race that; READ_ONCE
> + * keeps the value whole and the untag is best-effort.
> + */
>  static inline unsigned long __untagged_addr_remote(struct mm_struct *mm,
>                                                    unsigned long addr)
>  {
> -       mmap_assert_locked(mm);
> -       return addr & (mm)->context.untag_mask;
> +       return addr & READ_ONCE(mm->context.untag_mask);
>  }
>
> -#define untagged_addr_remote(mm, addr) ({                              \
> +#define untagged_addr_remote_unlocked(mm, addr)        ({                      \
>         unsigned long __addr = (__force unsigned long)(addr);           \
>         (__force __typeof__(addr))__untagged_addr_remote(mm, __addr);   \
>  })
>
> +#define untagged_addr_remote(mm, addr) ({                              \
> +       mmap_assert_locked(mm);                                         \
> +       untagged_addr_remote_unlocked(mm, addr);                        \
> +})
> +
>  #endif
>
>  #define valid_user_address(x) \
> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
> index d44afbe005bb..9fa659117f38 100644
> --- a/arch/x86/kernel/process_64.c
> +++ b/arch/x86/kernel/process_64.c
> @@ -814,7 +814,7 @@ static void enable_lam_func(void *__mm)
>  static void mm_enable_lam(struct mm_struct *mm)
>  {
>         mm->context.lam_cr3_mask = X86_CR3_LAM_U57;
> -       mm->context.untag_mask =  ~GENMASK(62, 57);
> +       WRITE_ONCE(mm->context.untag_mask, ~GENMASK(62, 57));
>
>         /*
>          * Even though the process must still be single-threaded at this
> @@ -952,7 +952,7 @@ long do_arch_prctl_64(struct task_struct *task, int option, unsigned long arg2)
>  #endif
>  #ifdef CONFIG_ADDRESS_MASKING
>         case ARCH_GET_UNTAG_MASK:
> -               return put_user(task->mm->context.untag_mask,
> +               return put_user(mm_untag_mask(task->mm),
>                                 (unsigned long __user *)arg2);
>         case ARCH_ENABLE_TAGGED_ADDR:
>                 return prctl_enable_tagged_addr(task->mm, arg2);
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index eddbbb65ccc4..7e6e4c89184c 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -34,6 +34,13 @@
>  })
>  #endif
>
> +#ifndef untagged_addr_remote_unlocked
> +#define untagged_addr_remote_unlocked(mm, addr)        ({      \
> +       (void)(mm);                                     \
> +       untagged_addr(addr);                            \
> +})
> +#endif
> +
>  #ifdef masked_user_access_begin
>   #define can_do_masked_user_access() 1
>  # ifndef masked_user_write_access_begin
>
> base-commit: 248951ddc14de84de3910f9b13f51491a8cd91df
> --
> 2.53.0-Meta
>