On 02/03/26 16:42, Christophe Leroy (CS GROUP) wrote:
> Hi Sayali,
>
> Le 28/02/2026 à 14:53, Sayali Patil a écrit :
>> On powerpc with PREEMPT_FULL or PREEMPT_LAZY and function tracing 
>> enabled,
>> KUAP warnings can be triggered from the VMX usercopy path under memory
>> stress workloads.
>>
>> KUAP requires that no subfunctions are called once userspace access has
>> been enabled. The existing VMX copy implementation violates this
>> requirement by invoking enter_vmx_usercopy() from the assembly path 
>> after
>> userspace access has already been enabled. If preemption occurs
>> in this window, the AMR state may not be preserved correctly,
>> leading to unexpected userspace access state and resulting in
>> KUAP warnings.
>>
>> Fix this by restructuring the VMX usercopy flow so that VMX selection
>> and VMX state management are centralized in raw_copy_tofrom_user(),
>> which is invoked by the raw_copy_{to,from,in}_user() wrappers.
>>
>> Introduce a usercopy_mode enum to describe the copy direction
>> (IN, FROM, TO) and use it to derive the required KUAP permissions.
>> Userspace access is now enabled and disabled through common helpers
>> based on the selected mode, ensuring that the correct read/write
>> permissions are applied consistently.
>>
>>   The new flow is:
>>
>>    - raw_copy_{to,from,in}_user() calls raw_copy_tofrom_user()
>>    - raw_copy_tofrom_user() decides whether to use the VMX path
>>      based on size and CPU capability
>>    - Call enter_vmx_usercopy() before enabling userspace access
>>    - Enable userspace access as per the usercopy mode
>>      and perform the VMX copy
>>    - Disable userspace access as per the usercopy mode
>>    - Call exit_vmx_usercopy()
>>    - Fall back to the base copy routine if the VMX copy faults
>>
>> With this change, the VMX assembly routines no longer perform VMX state
>> management or call helper functions; they only implement the
>> copy operations.
>> The previous feature-section based VMX selection inside
>> __copy_tofrom_user_power7() is removed, and a dedicated
>> __copy_tofrom_user_power7_vmx() entry point is introduced.
>>
>> This ensures correct KUAP ordering, avoids subfunction calls
>> while KUAP is unlocked, and eliminates the warnings while preserving
>> the VMX fast path.
>>
>> Fixes: de78a9c42a79 ("powerpc: Add a framework for Kernel Userspace 
>> Access Protection")
>> Reported-by: Shrikanth Hegde <sshegde@linux.ibm.com>
>> Closes: 
>> https://lore.kernel.org/all/20260109064917.777587-2-sshegde@linux.ibm.com/
>> Suggested-by: Christophe Leroy <chleroy@kernel.org>
>> Co-developed-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
>> Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
>> Signed-off-by: Sayali Patil <sayalip@linux.ibm.com>
>> ---
>>
>> v1->v2
>>    - Updated as per the review comments.
>>    - Centralized VMX usercopy handling in __copy_tofrom_user_vmx() in
>>      arch/powerpc/lib/vmx-helper.c.
>>    - Introduced a usercopy_mode enum to describe the copy direction
>>      (IN, FROM, TO) and derive the required KUAP permissions, avoiding
>>      duplication across the different usercopy paths.
>
> I like the reduction of duplication you propose but I can't see the 
> added value of that enum, what about:
>
> diff --git a/arch/powerpc/include/asm/uaccess.h 
> b/arch/powerpc/include/asm/uaccess.h
> index 63d6eb8b004e..14a3219db838 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -329,12 +329,6 @@ do {                                \
>  extern unsigned long __copy_tofrom_user(void __user *to,
>          const void __user *from, unsigned long size);
>
> -enum usercopy_mode {
> -    USERCOPY_IN,
> -    USERCOPY_FROM,
> -    USERCOPY_TO,
> -};
> -
>  unsigned long __copy_tofrom_user_vmx(void __user *to, const void 
> __user *from,
>                  unsigned long size, enum usercopy_mode mode);
>
> @@ -352,48 +346,18 @@ static inline bool will_use_vmx(unsigned long n)
>          n > VMX_COPY_THRESHOLD;
>  }
>
> -static inline void raw_copy_allow(void __user *to, enum usercopy_mode 
> mode)
> -{
> -    switch (mode) {
> -    case USERCOPY_IN:
> -        allow_user_access(to, KUAP_READ_WRITE);
> -        break;
> -    case USERCOPY_FROM:
> -        allow_user_access(NULL, KUAP_READ);
> -        break;
> -    case USERCOPY_TO:
> -        allow_user_access(to, KUAP_WRITE);
> -        break;
> -    }
> -}
> -
> -static inline void raw_copy_prevent(enum usercopy_mode mode)
> -{
> -    switch (mode) {
> -    case USERCOPY_IN:
> -        prevent_user_access(KUAP_READ_WRITE);
> -        break;
> -    case USERCOPY_FROM:
> -        prevent_user_access(KUAP_READ);
> -        break;
> -    case USERCOPY_TO:
> -        prevent_user_access(KUAP_WRITE);
> -        break;
> -    }
> -}
> -
>  static inline unsigned long raw_copy_tofrom_user(void __user *to,
>          const void __user *from, unsigned long n,
> -        enum usercopy_mode mode)
> +        unsigned long dir)
>  {
>      unsigned long ret;
>
>      if (will_use_vmx(n))
>          return __copy_tofrom_user_vmx(to, from,    n, mode);
>
> -    raw_copy_allow(to, mode);
> +    allow_user_access(to, dir);
>      ret = __copy_tofrom_user(to, from, n);
> -    raw_copy_prevent(mode);
> +    prevent_user_access(dir);
>      return ret;
>
>  }
> @@ -403,22 +367,20 @@ static inline unsigned long
>  raw_copy_in_user(void __user *to, const void __user *from, unsigned 
> long n)
>  {
>      barrier_nospec();
> -    return raw_copy_tofrom_user(to, from, n, USERCOPY_IN);
> +    return raw_copy_tofrom_user(to, from, n, KUAP_READ_WRITE);
>  }
>  #endif /* __powerpc64__ */
>
>  static inline unsigned long raw_copy_from_user(void *to,
>          const void __user *from, unsigned long n)
>  {
> -    return raw_copy_tofrom_user((__force void __user *)to, from,
> -                    n, USERCOPY_FROM);
> +    return raw_copy_tofrom_user((__force void __user *)to, from, n, 
> KUAP_READ);
>  }
>
>  static inline unsigned long
>  raw_copy_to_user(void __user *to, const void *from, unsigned long n)
>  {
> -    return raw_copy_tofrom_user(to, (__force const void __user *)from,
> -                    n, USERCOPY_TO);
> +    return raw_copy_tofrom_user(to, (__force const void __user 
> *)from, n, KUAP_WRITE);
>  }
>
>  unsigned long __arch_clear_user(void __user *addr, unsigned long size);
> diff --git a/arch/powerpc/lib/vmx-helper.c 
> b/arch/powerpc/lib/vmx-helper.c
> index 35080885204b..4610f7153fd9 100644
> --- a/arch/powerpc/lib/vmx-helper.c
> +++ b/arch/powerpc/lib/vmx-helper.c
> @@ -11,25 +11,25 @@
>  #include <asm/switch_to.h>
>
>  unsigned long __copy_tofrom_user_vmx(void __user *to, const void 
> __user *from,
> -            unsigned long size, enum usercopy_mode mode)
> +            unsigned long size, unsigned long dir)
>  {
>      unsigned long ret;
>
>      if (!enter_vmx_usercopy()) {
> -        raw_copy_allow(to, mode);
> +        allow_user_access(to, dir);
>          ret = __copy_tofrom_user(to, from, size);
> -        raw_copy_prevent(mode);
> +        prevent_user_access(dir);
>          return ret;
>      }
>
> -    raw_copy_allow(to, mode);
> +    allow_user_access(to, dir);
>      ret = __copy_tofrom_user_power7_vmx(to, from, size);
> -    raw_copy_prevent(mode);
> +    prevent_user_access(dir);
>      exit_vmx_usercopy();
>      if (unlikely(ret)) {
> -        raw_copy_allow(to, mode);
> +        allow_user_access(to, dir);
>          ret = __copy_tofrom_user_base(to, from, size);
> -        raw_copy_prevent(mode);
> +        prevent_user_access(dir);
>      }
>
>      return ret;
>
>
>
> Christophe
>
>
Hi Christophe,
Thanks for the review.
With the suggested change, we are hitting a compilation error.

The issue is related to how KUAP enforces the access direction.
allow_user_access() contains:

BUILD_BUG_ON(!__builtin_constant_p(dir));

which requires that the access direction is a compile-time constant.
If we pass a runtime value (for example, an unsigned long), the
__builtin_constant_p() check fails and triggers the following build error.

Error:
In function 'allow_user_access', inlined from '__copy_tofrom_user_vmx' 
at arch/powerpc/lib/vmx-helper.c:19:3:
BUILD_BUG_ON failed: !__builtin_constant_p(dir) 706


The previous implementation worked because allow_user_access() was 
invoked with enum
constants (READ, WRITE, READ_WRITE), which satisfied the 
__builtin_constant_p() requirement.
So in this case, the function must be called with a compile-time 
constant to satisfy KUAP.

Please let me know if you would prefer a different approach.

Regards,
Sayali


>>
>> v1: 
>> https://lore.kernel.org/all/20260217124457.89219-1-sayalip@linux.ibm.com/
>>
>> ---
>>   arch/powerpc/include/asm/uaccess.h | 95 ++++++++++++++++++++++++------
>>   arch/powerpc/lib/copyuser_64.S     |  1 +
>>   arch/powerpc/lib/copyuser_power7.S | 45 +++++---------
>>   arch/powerpc/lib/vmx-helper.c      | 26 ++++++++
>>   4 files changed, 119 insertions(+), 48 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/uaccess.h 
>> b/arch/powerpc/include/asm/uaccess.h
>> index ba1d878c3f40..63d6eb8b004e 100644
>> --- a/arch/powerpc/include/asm/uaccess.h
>> +++ b/arch/powerpc/include/asm/uaccess.h
>> @@ -15,6 +15,9 @@
>>   #define TASK_SIZE_MAX        TASK_SIZE_USER64
>>   #endif
>>   +/* Threshold above which VMX copy path is used */
>> +#define VMX_COPY_THRESHOLD 3328
>> +
>>   #include <asm-generic/access_ok.h>
>>     /*
>> @@ -326,40 +329,96 @@ do {                                \
>>   extern unsigned long __copy_tofrom_user(void __user *to,
>>           const void __user *from, unsigned long size);
>>   -#ifdef __powerpc64__
>> -static inline unsigned long
>> -raw_copy_in_user(void __user *to, const void __user *from, unsigned 
>> long n)
>> +enum usercopy_mode {
>> +    USERCOPY_IN,
>> +    USERCOPY_FROM,
>> +    USERCOPY_TO,
>> +};
>> +
>> +unsigned long __copy_tofrom_user_vmx(void __user *to, const void 
>> __user *from,
>> +                unsigned long size, enum usercopy_mode mode);
>> +
>> +unsigned long __copy_tofrom_user_base(void __user *to,
>> +        const void __user *from, unsigned long size);
>> +
>> +unsigned long __copy_tofrom_user_power7_vmx(void __user *to,
>> +        const void __user *from, unsigned long size);
>> +
>> +
>> +static inline bool will_use_vmx(unsigned long n)
>> +{
>> +    return IS_ENABLED(CONFIG_ALTIVEC) &&
>> +        cpu_has_feature(CPU_FTR_VMX_COPY) &&
>> +        n > VMX_COPY_THRESHOLD;
>> +}
>> +
>> +static inline void raw_copy_allow(void __user *to, enum 
>> usercopy_mode mode)
>> +{
>> +    switch (mode) {
>> +    case USERCOPY_IN:
>> +        allow_user_access(to, KUAP_READ_WRITE);
>> +        break;
>> +    case USERCOPY_FROM:
>> +        allow_user_access(NULL, KUAP_READ);
>> +        break;
>> +    case USERCOPY_TO:
>> +        allow_user_access(to, KUAP_WRITE);
>> +        break;
>> +    }
>> +}
>> +
>> +static inline void raw_copy_prevent(enum usercopy_mode mode)
>> +{
>> +    switch (mode) {
>> +    case USERCOPY_IN:
>> +        prevent_user_access(KUAP_READ_WRITE);
>> +        break;
>> +    case USERCOPY_FROM:
>> +        prevent_user_access(KUAP_READ);
>> +        break;
>> +    case USERCOPY_TO:
>> +        prevent_user_access(KUAP_WRITE);
>> +        break;
>> +    }
>> +}
>> +
>> +static inline unsigned long raw_copy_tofrom_user(void __user *to,
>> +        const void __user *from, unsigned long n,
>> +        enum usercopy_mode mode)
>>   {
>>       unsigned long ret;
>>   -    barrier_nospec();
>> -    allow_user_access(to, KUAP_READ_WRITE);
>> +    if (will_use_vmx(n))
>> +        return __copy_tofrom_user_vmx(to, from,    n, mode);
>> +
>> +    raw_copy_allow(to, mode);
>>       ret = __copy_tofrom_user(to, from, n);
>> -    prevent_user_access(KUAP_READ_WRITE);
>> +    raw_copy_prevent(mode);
>>       return ret;
>> +
>> +}
>> +
>> +#ifdef __powerpc64__
>> +static inline unsigned long
>> +raw_copy_in_user(void __user *to, const void __user *from, unsigned 
>> long n)
>> +{
>> +    barrier_nospec();
>> +    return raw_copy_tofrom_user(to, from, n, USERCOPY_IN);
>>   }
>>   #endif /* __powerpc64__ */
>>     static inline unsigned long raw_copy_from_user(void *to,
>>           const void __user *from, unsigned long n)
>>   {
>> -    unsigned long ret;
>> -
>> -    allow_user_access(NULL, KUAP_READ);
>> -    ret = __copy_tofrom_user((__force void __user *)to, from, n);
>> -    prevent_user_access(KUAP_READ);
>> -    return ret;
>> +    return raw_copy_tofrom_user((__force void __user *)to, from,
>> +                    n, USERCOPY_FROM);
>>   }
>>     static inline unsigned long
>>   raw_copy_to_user(void __user *to, const void *from, unsigned long n)
>>   {
>> -    unsigned long ret;
>> -
>> -    allow_user_access(to, KUAP_WRITE);
>> -    ret = __copy_tofrom_user(to, (__force const void __user *)from, n);
>> -    prevent_user_access(KUAP_WRITE);
>> -    return ret;
>> +    return raw_copy_tofrom_user(to, (__force const void __user *)from,
>> +                    n, USERCOPY_TO);
>>   }
>>     unsigned long __arch_clear_user(void __user *addr, unsigned long 
>> size);
>> diff --git a/arch/powerpc/lib/copyuser_64.S 
>> b/arch/powerpc/lib/copyuser_64.S
>> index 9af969d2cc0c..25a99108caff 100644
>> --- a/arch/powerpc/lib/copyuser_64.S
>> +++ b/arch/powerpc/lib/copyuser_64.S
>> @@ -562,3 +562,4 @@ exc;    std    r10,32(3)
>>       li    r5,4096
>>       b    .Ldst_aligned
>>   EXPORT_SYMBOL(__copy_tofrom_user)
>> +EXPORT_SYMBOL(__copy_tofrom_user_base)
>> diff --git a/arch/powerpc/lib/copyuser_power7.S 
>> b/arch/powerpc/lib/copyuser_power7.S
>> index 8474c682a178..17dbcfbae25f 100644
>> --- a/arch/powerpc/lib/copyuser_power7.S
>> +++ b/arch/powerpc/lib/copyuser_power7.S
>> @@ -5,13 +5,9 @@
>>    *
>>    * Author: Anton Blanchard <anton@au.ibm.com>
>>    */
>> +#include <linux/export.h>
>>   #include <asm/ppc_asm.h>
>>   -#ifndef SELFTEST_CASE
>> -/* 0 == don't use VMX, 1 == use VMX */
>> -#define SELFTEST_CASE    0
>> -#endif
>> -
>>   #ifdef __BIG_ENDIAN__
>>   #define LVS(VRT,RA,RB)        lvsl    VRT,RA,RB
>>   #define VPERM(VRT,VRA,VRB,VRC)    vperm    VRT,VRA,VRB,VRC
>> @@ -47,10 +43,14 @@
>>       ld    r15,STK_REG(R15)(r1)
>>       ld    r14,STK_REG(R14)(r1)
>>   .Ldo_err3:
>> -    bl    CFUNC(exit_vmx_usercopy)
>> +    ld      r6,STK_REG(R31)(r1)    /* original destination pointer */
>> +    ld      r5,STK_REG(R29)(r1)    /* original number of bytes */
>> +    subf    r7,r6,r3        /* #bytes copied */
>> +    subf    r3,r7,r5        /* #bytes not copied in r3 */
>>       ld    r0,STACKFRAMESIZE+16(r1)
>>       mtlr    r0
>> -    b    .Lexit
>> +    addi    r1,r1,STACKFRAMESIZE
>> +    blr
>>   #endif /* CONFIG_ALTIVEC */
>>     .Ldo_err2:
>> @@ -74,7 +74,6 @@
>>     _GLOBAL(__copy_tofrom_user_power7)
>>       cmpldi    r5,16
>> -    cmpldi    cr1,r5,3328
>>         std    r3,-STACKFRAMESIZE+STK_REG(R31)(r1)
>>       std    r4,-STACKFRAMESIZE+STK_REG(R30)(r1)
>> @@ -82,12 +81,6 @@ _GLOBAL(__copy_tofrom_user_power7)
>>         blt    .Lshort_copy
>>   -#ifdef CONFIG_ALTIVEC
>> -test_feature = SELFTEST_CASE
>> -BEGIN_FTR_SECTION
>> -    bgt    cr1,.Lvmx_copy
>> -END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
>> -#endif
>>     .Lnonvmx_copy:
>>       /* Get the source 8B aligned */
>> @@ -263,23 +256,14 @@ err1;    stb    r0,0(r3)
>>   15:    li    r3,0
>>       blr
>>   -.Lunwind_stack_nonvmx_copy:
>> -    addi    r1,r1,STACKFRAMESIZE
>> -    b    .Lnonvmx_copy
>> -
>> -.Lvmx_copy:
>>   #ifdef CONFIG_ALTIVEC
>> +_GLOBAL(__copy_tofrom_user_power7_vmx)
>>       mflr    r0
>>       std    r0,16(r1)
>>       stdu    r1,-STACKFRAMESIZE(r1)
>> -    bl    CFUNC(enter_vmx_usercopy)
>> -    cmpwi    cr1,r3,0
>> -    ld    r0,STACKFRAMESIZE+16(r1)
>> -    ld    r3,STK_REG(R31)(r1)
>> -    ld    r4,STK_REG(R30)(r1)
>> -    ld    r5,STK_REG(R29)(r1)
>> -    mtlr    r0
>>   +    std     r3,STK_REG(R31)(r1)
>> +    std     r5,STK_REG(R29)(r1)
>>       /*
>>        * We prefetch both the source and destination using enhanced 
>> touch
>>        * instructions. We use a stream ID of 0 for the load side and
>> @@ -300,8 +284,6 @@ err1;    stb    r0,0(r3)
>>         DCBT_SETUP_STREAMS(r6, r7, r9, r10, r8)
>>   -    beq    cr1,.Lunwind_stack_nonvmx_copy
>> -
>>       /*
>>        * If source and destination are not relatively aligned we use a
>>        * slower permute loop.
>> @@ -478,7 +460,8 @@ err3;    lbz    r0,0(r4)
>>   err3;    stb    r0,0(r3)
>>     15:    addi    r1,r1,STACKFRAMESIZE
>> -    b    CFUNC(exit_vmx_usercopy)    /* tail call optimise */
>> +    li r3,0
>> +    blr
>>     .Lvmx_unaligned_copy:
>>       /* Get the destination 16B aligned */
>> @@ -681,5 +664,7 @@ err3;    lbz    r0,0(r4)
>>   err3;    stb    r0,0(r3)
>>     15:    addi    r1,r1,STACKFRAMESIZE
>> -    b    CFUNC(exit_vmx_usercopy)    /* tail call optimise */
>> +    li r3,0
>> +    blr
>> +EXPORT_SYMBOL(__copy_tofrom_user_power7_vmx)
>>   #endif /* CONFIG_ALTIVEC */
>> diff --git a/arch/powerpc/lib/vmx-helper.c 
>> b/arch/powerpc/lib/vmx-helper.c
>> index 54340912398f..35080885204b 100644
>> --- a/arch/powerpc/lib/vmx-helper.c
>> +++ b/arch/powerpc/lib/vmx-helper.c
>> @@ -10,6 +10,32 @@
>>   #include <linux/hardirq.h>
>>   #include <asm/switch_to.h>
>>   +unsigned long __copy_tofrom_user_vmx(void __user *to, const void 
>> __user *from,
>> +            unsigned long size, enum usercopy_mode mode)
>> +{
>> +    unsigned long ret;
>> +
>> +    if (!enter_vmx_usercopy()) {
>> +        raw_copy_allow(to, mode);
>> +        ret = __copy_tofrom_user(to, from, size);
>> +        raw_copy_prevent(mode);
>> +        return ret;
>> +    }
>> +
>> +    raw_copy_allow(to, mode);
>> +    ret = __copy_tofrom_user_power7_vmx(to, from, size);
>> +    raw_copy_prevent(mode);
>> +    exit_vmx_usercopy();
>> +    if (unlikely(ret)) {
>> +        raw_copy_allow(to, mode);
>> +        ret = __copy_tofrom_user_base(to, from, size);
>> +        raw_copy_prevent(mode);
>> +    }
>> +
>> +    return ret;
>> +}
>> +EXPORT_SYMBOL(__copy_tofrom_user_vmx);
>> +
>>   int enter_vmx_usercopy(void)
>>   {
>>       if (in_interrupt())
>