Re: [PATCH] riscv: futex: untag the user pointer before the atomic access

[Samuel Holland]

On 2026-10-01 8:43 PM, Ziyi Guo wrote:
> access_ok() checks untagged_addr(uaddr), but arch_futex_atomic_op_inuser()
> and futex_atomic_cmpxchg_inatomic() hand the raw uaddr to the inline asm
> amoswap/amoadd/.../lr.w+sc.w through the "+m" (*uaddr) operand.  When the
> tagged address ABI is enabled (CONFIG_RISCV_ISA_SUPM, prctl
> PR_SET_TAGGED_ADDR_CTRL with PMLEN != 0), those two addresses differ: a
> user pointer whose top PMLEN bits hold a tag passes access_ok() after
> untagging, but the atomic is then performed on the still-tagged raw
> address.
> 
> Supervisor-mode data accesses are not subject to the U-mode pointer
> masking (that is governed by menvcfg.PMM, which the kernel does not set),
> so hardware does not strip the tag for the kernel's own access.  With
> Sv57 and PMLEN=16 the tag bits overlap the canonical-address bits, so a
> tagged pointer can name a canonical kernel virtual address (e.g. in the
> linear map) whose untagged form is a valid user address.  An unprivileged

Thankfully, the combination Sv57 + PMLEN=16 is unlikely to be in use because the
address bit overlap tends to create performance problems.

> process can thus make FUTEX_WAKE_OP perform an atomic read-modify-write on
> an arbitrary kernel address, with the matching FUTEX_OP_CMP_* result
> serving as a read oracle.
> 
> get_user()/put_user()/raw_copy_{to,from}_user() already untag the pointer
> after the access_ok() check; do the same in the futex helpers so the
> atomic operates on the address that was actually validated.

> Fixes: 2e1743085887 ("riscv: Add support for the tagged address ABI")
> Signed-off-by: Ziyi Guo <guoziyi114@xxxxxxxxx>
> ---
>  arch/riscv/include/asm/futex.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/futex.h b/arch/riscv/include/asm/futex.h
> index 90c86b115e00..8d80e580eec5 100644
> --- a/arch/riscv/include/asm/futex.h
> +++ b/arch/riscv/include/asm/futex.h
> @@ -40,6 +40,7 @@ arch_futex_atomic_op_inuser(int op, int oparg, int *oval, u32 __user *uaddr)
>  
>  	if (!access_ok(uaddr, sizeof(u32)))
>  		return -EFAULT;
> +	uaddr = untagged_addr(uaddr);   /* FIX: operate on the checked address */
>  
>  	switch (op) {
>  	case FUTEX_OP_SET:
> @@ -82,6 +83,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32 __user *uaddr,
>  
>  	if (!access_ok(uaddr, sizeof(u32)))
>  		return -EFAULT;
> +	uaddr = untagged_addr(uaddr);   /* FIX: operate on the checked address */

The comments are not necessary. The reason for the change is in the commit log.
With them removed:

Reviewed-by: Samuel Holland <samuel.holland@xxxxxxxxxx>

>  
>  	__enable_user_access();
>  	__asm__ __volatile__ (