Re: [PATCH 1/3] x86: fix access_ok() and valid_user_address() using wrong USER_PTR_MAX in modules

[David Laight <david.laight.linux@gmail.com>]

On Fri, 31 Oct 2025 18:42:18 +0100
Mateusz Guzik <mjguzik@gmail.com> wrote:

> [real commit message will land here later]

Hmmm... modules use the 0x123456789abcdef0 placeholder (the 0 might not be
in the right place), this is non-canonical so nothing is badly broken.
Just allows speculative accesses to kernel space on some cpu.

> ---
>  arch/x86/include/asm/uaccess_64.h | 17 +++++++++--------
>  arch/x86/kernel/cpu/common.c      |  8 +++++---
>  2 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h
> index c8a5ae35c871..f60c0ed147c3 100644
> --- a/arch/x86/include/asm/uaccess_64.h
> +++ b/arch/x86/include/asm/uaccess_64.h
> @@ -12,13 +12,14 @@
>  #include <asm/cpufeatures.h>
>  #include <asm/page.h>
>  #include <asm/percpu.h>
> -#include <asm/runtime-const.h>
>  
> -/*
> - * Virtual variable: there's no actual backing store for this,
> - * it can purely be used as 'runtime_const_ptr(USER_PTR_MAX)'
> - */
> -extern unsigned long USER_PTR_MAX;
> +extern unsigned long user_ptr_max;
> +#ifdef MODULE
> +#define __user_ptr_max_accessor	user_ptr_max
> +#else
> +#include <asm/runtime-const.h>
> +#define __user_ptr_max_accessor	runtime_const_ptr(user_ptr_max)
> +#endif
>  
>  #ifdef CONFIG_ADDRESS_MASKING
>  /*
> @@ -54,7 +55,7 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm,
>  #endif
>  
>  #define valid_user_address(x) \
> -	likely((__force unsigned long)(x) <= runtime_const_ptr(USER_PTR_MAX))
> +	likely((__force unsigned long)(x) <= __user_ptr_max_accessor)
>  
>  /*
>   * Masking the user address is an alternative to a conditional
> @@ -67,7 +68,7 @@ static inline void __user *mask_user_address(const void __user *ptr)
>  	asm("cmp %1,%0\n\t"
>  	    "cmova %1,%0"
>  		:"=r" (ret)
> -		:"r" (runtime_const_ptr(USER_PTR_MAX)),
> +		:"r" (__user_ptr_max_accessor),
>  		 "0" (ptr));
>  	return ret;
>  }
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 3ff9682d8bc4..f338f5e9adfc 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -78,6 +78,9 @@
>  DEFINE_PER_CPU_READ_MOSTLY(struct cpuinfo_x86, cpu_info);
>  EXPORT_PER_CPU_SYMBOL(cpu_info);
>  
> +unsigned long user_ptr_max __ro_after_init;
> +EXPORT_SYMBOL(user_ptr_max);

That doesn't appear to be inside a CONFIG_X86_64 define.
I think I'd initialise it to one of its two values - probably the LA48 one.

	David

> +
>  u32 elf_hwcap2 __read_mostly;
>  
>  /* Number of siblings per CPU package */
> @@ -2575,14 +2578,13 @@ void __init arch_cpu_finalize_init(void)
>  	alternative_instructions();
>  
>  	if (IS_ENABLED(CONFIG_X86_64)) {
> -		unsigned long USER_PTR_MAX = TASK_SIZE_MAX;
> -
> +		user_ptr_max = TASK_SIZE_MAX;
>  		/*
>  		 * Enable this when LAM is gated on LASS support
>  		if (cpu_feature_enabled(X86_FEATURE_LAM))
>  			USER_PTR_MAX = (1ul << 63) - PAGE_SIZE;
>  		 */
> -		runtime_const_init(ptr, USER_PTR_MAX);
> +		runtime_const_init(ptr, user_ptr_max);
>  
>  		/*
>  		 * Make sure the first 2MB area is not mapped by huge pages