[PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation

Lukas Gerlach posted 2 patches 1 month, 3 weeks ago
[PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation
Posted by Lukas Gerlach 1 month, 3 weeks ago
Similarly to x86 and arm64, mitigate speculation past an access_ok()
check by masking the pointer before use.

On RISC-V, user addresses have the MSB clear while kernel addresses
have the MSB set. The uaccess_mask_ptr() function clears the MSB,
ensuring any kernel pointer becomes invalid and will fault, while
valid user pointers remain unchanged. This prevents speculative
access to kernel memory via user copy functions.

The masking is applied to __get_user, __put_user, raw_copy_from_user,
raw_copy_to_user, clear_user, and the unsafe_* variants.

Signed-off-by: Lukas Gerlach <lukas.gerlach@cispa.de>
---
 arch/riscv/include/asm/uaccess.h | 41 +++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
index 36bba6720c26..ceee1d62ff9b 100644
--- a/arch/riscv/include/asm/uaccess.h
+++ b/arch/riscv/include/asm/uaccess.h
@@ -74,6 +74,23 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm, unsigne
 #define __typefits(x, type, not) \
 	__builtin_choose_expr(sizeof(x) <= sizeof(type), (unsigned type)0, not)
 
+/*
+ * Sanitize a uaccess pointer such that it cannot reach any kernel address.
+ *
+ * On RISC-V, virtual addresses are sign-extended from the top implemented bit.
+ * User addresses have the MSB clear; kernel addresses have the MSB set.
+ * Clearing the MSB ensures any kernel pointer becomes non-canonical and will
+ * fault, while valid user pointers remain unchanged.
+ */
+#define uaccess_mask_ptr(ptr) ((__typeof__(ptr))__uaccess_mask_ptr(ptr))
+static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
+{
+	unsigned long val = (unsigned long)ptr;
+
+	val = (val << 1) >> 1;
+	return (void __user *)val;
+}
+
 /*
  * The exception table consists of pairs of addresses: the first is the
  * address of an instruction that is allowed to fault, and the second is
@@ -235,7 +252,8 @@ __gu_failed:								\
  */
 #define __get_user(x, ptr)					\
 ({								\
-	const __typeof__(*(ptr)) __user *__gu_ptr = untagged_addr(ptr); \
+	const __typeof__(*(ptr)) __user *__gu_ptr =		\
+		uaccess_mask_ptr(untagged_addr(ptr));		\
 	long __gu_err = 0;					\
 	__typeof__(x) __gu_val;					\
 								\
@@ -366,7 +384,8 @@ err_label:							\
  */
 #define __put_user(x, ptr)					\
 ({								\
-	__typeof__(*(ptr)) __user *__gu_ptr = untagged_addr(ptr); \
+	__typeof__(*(ptr)) __user *__gu_ptr =			\
+		uaccess_mask_ptr(untagged_addr(ptr));		\
 	__typeof__(*__gu_ptr) __val = (x);			\
 	long __pu_err = 0;					\
 								\
@@ -413,13 +432,15 @@ unsigned long __must_check __asm_copy_from_user(void *to,
 static inline unsigned long
 raw_copy_from_user(void *to, const void __user *from, unsigned long n)
 {
-	return __asm_copy_from_user(to, untagged_addr(from), n);
+	return __asm_copy_from_user(to,
+		uaccess_mask_ptr(untagged_addr(from)), n);
 }
 
 static inline unsigned long
 raw_copy_to_user(void __user *to, const void *from, unsigned long n)
 {
-	return __asm_copy_to_user(untagged_addr(to), from, n);
+	return __asm_copy_to_user(
+		uaccess_mask_ptr(untagged_addr(to)), from, n);
 }
 
 extern long strncpy_from_user(char *dest, const char __user *src, long count);
@@ -434,7 +455,7 @@ unsigned long __must_check clear_user(void __user *to, unsigned long n)
 {
 	might_fault();
 	return access_ok(to, n) ?
-		__clear_user(untagged_addr(to), n) : n;
+		__clear_user(uaccess_mask_ptr(untagged_addr(to)), n) : n;
 }
 
 #define arch_get_kernel_nofault(dst, src, type, err_label)			\
@@ -461,20 +482,22 @@ static inline void user_access_restore(unsigned long enabled) { }
  * the error labels - thus the macro games.
  */
 #define arch_unsafe_put_user(x, ptr, label)				\
-	__put_user_nocheck(x, (ptr), label)
+	__put_user_nocheck(x, uaccess_mask_ptr(ptr), label)
 
 #define arch_unsafe_get_user(x, ptr, label)	do {			\
 	__inttype(*(ptr)) __gu_val;					\
-	__get_user_nocheck(__gu_val, (ptr), label);			\
+	__get_user_nocheck(__gu_val, uaccess_mask_ptr(ptr), label);	\
 	(x) = (__force __typeof__(*(ptr)))__gu_val;			\
 } while (0)
 
 #define unsafe_copy_to_user(_dst, _src, _len, label)			\
-	if (__asm_copy_to_user_sum_enabled(_dst, _src, _len))		\
+	if (__asm_copy_to_user_sum_enabled(				\
+		uaccess_mask_ptr(_dst), _src, _len))			\
 		goto label;
 
 #define unsafe_copy_from_user(_dst, _src, _len, label)			\
-	if (__asm_copy_from_user_sum_enabled(_dst, _src, _len))		\
+	if (__asm_copy_from_user_sum_enabled(				\
+		_dst, uaccess_mask_ptr(_src), _len))			\
 		goto label;
 
 #else /* CONFIG_MMU */
-- 
2.51.0
Re: [PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation
Posted by Deepak Gupta 1 month, 3 weeks ago
On Thu, Dec 18, 2025 at 08:13:31PM +0100, Lukas Gerlach wrote:
>Similarly to x86 and arm64, mitigate speculation past an access_ok()
>check by masking the pointer before use.
>
>On RISC-V, user addresses have the MSB clear while kernel addresses
>have the MSB set. The uaccess_mask_ptr() function clears the MSB,
>ensuring any kernel pointer becomes invalid and will fault, while
>valid user pointers remain unchanged. This prevents speculative
>access to kernel memory via user copy functions.
>
>The masking is applied to __get_user, __put_user, raw_copy_from_user,
>raw_copy_to_user, clear_user, and the unsafe_* variants.
>
>Signed-off-by: Lukas Gerlach <lukas.gerlach@cispa.de>
>---
> arch/riscv/include/asm/uaccess.h | 41 +++++++++++++++++++++++++-------
> 1 file changed, 32 insertions(+), 9 deletions(-)
>
>diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
>index 36bba6720c26..ceee1d62ff9b 100644
>--- a/arch/riscv/include/asm/uaccess.h
>+++ b/arch/riscv/include/asm/uaccess.h
>@@ -74,6 +74,23 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm, unsigne
> #define __typefits(x, type, not) \
> 	__builtin_choose_expr(sizeof(x) <= sizeof(type), (unsigned type)0, not)
>
>+/*
>+ * Sanitize a uaccess pointer such that it cannot reach any kernel address.
>+ *
>+ * On RISC-V, virtual addresses are sign-extended from the top implemented bit.
>+ * User addresses have the MSB clear; kernel addresses have the MSB set.
>+ * Clearing the MSB ensures any kernel pointer becomes non-canonical and will
>+ * fault, while valid user pointers remain unchanged.
>+ */
>+#define uaccess_mask_ptr(ptr) ((__typeof__(ptr))__uaccess_mask_ptr(ptr))
>+static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
>+{
>+	unsigned long val = (unsigned long)ptr;
>+
>+	val = (val << 1) >> 1;
>+	return (void __user *)val;

This is only clearing b63 which is what we don't need here.

You should be clearing b47 (if bit indexing starts at 0) on Sv48 and b56
on Sv57 system.

Anything above b47/b56 isn't going to be used anyways in indexing into
page tables and will be ignored if pointer masking is enabled at S.

>+}
>+
> /*
>  * The exception table consists of pairs of addresses: the first is the
>  * address of an instruction that is allowed to fault, and the second is
>@@ -235,7 +252,8 @@ __gu_failed:								\
>  */
> #define __get_user(x, ptr)					\
> ({								\
>-	const __typeof__(*(ptr)) __user *__gu_ptr = untagged_addr(ptr); \
>+	const __typeof__(*(ptr)) __user *__gu_ptr =		\
>+		uaccess_mask_ptr(untagged_addr(ptr));		\
> 	long __gu_err = 0;					\
> 	__typeof__(x) __gu_val;					\
> 								\
>@@ -366,7 +384,8 @@ err_label:							\
>  */
> #define __put_user(x, ptr)					\
> ({								\
>-	__typeof__(*(ptr)) __user *__gu_ptr = untagged_addr(ptr); \
>+	__typeof__(*(ptr)) __user *__gu_ptr =			\
>+		uaccess_mask_ptr(untagged_addr(ptr));		\
> 	__typeof__(*__gu_ptr) __val = (x);			\
> 	long __pu_err = 0;					\
> 								\
>@@ -413,13 +432,15 @@ unsigned long __must_check __asm_copy_from_user(void *to,
> static inline unsigned long
> raw_copy_from_user(void *to, const void __user *from, unsigned long n)
> {
>-	return __asm_copy_from_user(to, untagged_addr(from), n);
>+	return __asm_copy_from_user(to,
>+		uaccess_mask_ptr(untagged_addr(from)), n);
> }
>
> static inline unsigned long
> raw_copy_to_user(void __user *to, const void *from, unsigned long n)
> {
>-	return __asm_copy_to_user(untagged_addr(to), from, n);
>+	return __asm_copy_to_user(
>+		uaccess_mask_ptr(untagged_addr(to)), from, n);
> }
>
> extern long strncpy_from_user(char *dest, const char __user *src, long count);
>@@ -434,7 +455,7 @@ unsigned long __must_check clear_user(void __user *to, unsigned long n)
> {
> 	might_fault();
> 	return access_ok(to, n) ?
>-		__clear_user(untagged_addr(to), n) : n;
>+		__clear_user(uaccess_mask_ptr(untagged_addr(to)), n) : n;
> }
>
> #define arch_get_kernel_nofault(dst, src, type, err_label)			\
>@@ -461,20 +482,22 @@ static inline void user_access_restore(unsigned long enabled) { }
>  * the error labels - thus the macro games.
>  */
> #define arch_unsafe_put_user(x, ptr, label)				\
>-	__put_user_nocheck(x, (ptr), label)
>+	__put_user_nocheck(x, uaccess_mask_ptr(ptr), label)
>
> #define arch_unsafe_get_user(x, ptr, label)	do {			\
> 	__inttype(*(ptr)) __gu_val;					\
>-	__get_user_nocheck(__gu_val, (ptr), label);			\
>+	__get_user_nocheck(__gu_val, uaccess_mask_ptr(ptr), label);	\
> 	(x) = (__force __typeof__(*(ptr)))__gu_val;			\
> } while (0)
>
> #define unsafe_copy_to_user(_dst, _src, _len, label)			\
>-	if (__asm_copy_to_user_sum_enabled(_dst, _src, _len))		\
>+	if (__asm_copy_to_user_sum_enabled(				\
>+		uaccess_mask_ptr(_dst), _src, _len))			\
> 		goto label;
>
> #define unsafe_copy_from_user(_dst, _src, _len, label)			\
>-	if (__asm_copy_from_user_sum_enabled(_dst, _src, _len))		\
>+	if (__asm_copy_from_user_sum_enabled(				\
>+		_dst, uaccess_mask_ptr(_src), _len))			\
> 		goto label;
>
> #else /* CONFIG_MMU */
>-- 
>2.51.0
>
>
Re: [PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation
Posted by David Laight 1 month, 1 week ago
On Fri, 19 Dec 2025 16:44:11 -0800
Deepak Gupta <debug@rivosinc.com> wrote:

> On Thu, Dec 18, 2025 at 08:13:31PM +0100, Lukas Gerlach wrote:
> >Similarly to x86 and arm64, mitigate speculation past an access_ok()
> >check by masking the pointer before use.
> >
> >On RISC-V, user addresses have the MSB clear while kernel addresses
> >have the MSB set. The uaccess_mask_ptr() function clears the MSB,
> >ensuring any kernel pointer becomes invalid and will fault, while
> >valid user pointers remain unchanged. This prevents speculative
> >access to kernel memory via user copy functions.
> >
> >The masking is applied to __get_user, __put_user, raw_copy_from_user,
> >raw_copy_to_user, clear_user, and the unsafe_* variants.
> >
> >Signed-off-by: Lukas Gerlach <lukas.gerlach@cispa.de>
> >---
> > arch/riscv/include/asm/uaccess.h | 41 +++++++++++++++++++++++++-------
> > 1 file changed, 32 insertions(+), 9 deletions(-)
> >
> >diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
> >index 36bba6720c26..ceee1d62ff9b 100644
> >--- a/arch/riscv/include/asm/uaccess.h
> >+++ b/arch/riscv/include/asm/uaccess.h
> >@@ -74,6 +74,23 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm, unsigne
> > #define __typefits(x, type, not) \
> > 	__builtin_choose_expr(sizeof(x) <= sizeof(type), (unsigned type)0, not)
> >
> >+/*
> >+ * Sanitize a uaccess pointer such that it cannot reach any kernel address.
> >+ *
> >+ * On RISC-V, virtual addresses are sign-extended from the top implemented bit.
> >+ * User addresses have the MSB clear; kernel addresses have the MSB set.
> >+ * Clearing the MSB ensures any kernel pointer becomes non-canonical and will
> >+ * fault, while valid user pointers remain unchanged.
> >+ */
> >+#define uaccess_mask_ptr(ptr) ((__typeof__(ptr))__uaccess_mask_ptr(ptr))
> >+static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
> >+{
> >+	unsigned long val = (unsigned long)ptr;
> >+
> >+	val = (val << 1) >> 1;
> >+	return (void __user *)val;  
> 
> This is only clearing b63 which is what we don't need here.

It is also entirely the wrong operation.
A kernel address needs converting into an address that is guaranteed
to fault - not a user address that might be valid.

You need a guard page of address space between valid user addresses
and valid kernel addresses that is guaranteed to not be used.
Typically this is the top page of the user address space.
All kernel addresses need converting to the base of the guard page
using ALU operations (to avoid speculative accesses).
So: ptr = min(ptr, guard_page); easiest done (as in x86-64) with
a compare and conditional move.
The ppc version is more complex because there isn't a usable conditional
move instruction.
I think this would work for x86-32:
	offset = ptr + -guard_page;
	mask = sbb(x,x); // ~0u if the add set the carry flag.
	ptr -= offset & mask;
You need to find something that works for riscv.

> 
> You should be clearing b47 (if bit indexing starts at 0) on Sv48 and b56
> on Sv57 system.
> 
> Anything above b47/b56 isn't going to be used anyways in indexing into
> page tables and will be ignored if pointer masking is enabled at S.

Gah more broken hardware...
Ignoring the high address bits doesn't work and is really a bad idea.
Arm did it as well.
Trying to do 'user pointer masking' for uaccess validation is a PITA
if you have to allow for non-zero values in the high bits it makes
life even more complicated. 

And 'pointer masking' is so broken unless it is done at the page level
and enforced by the hardware.
What it does is make random/invalid addresses much more likely to be valid.
An interpreter can use them internally, but they are so slow software
masking  (following the validation) shouldn't be a real issue.

	David
Re: [PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation
Posted by Deepak Gupta 1 month, 1 week ago
On Sat, Dec 27, 2025 at 09:28:59PM +0000, David Laight wrote:
>On Fri, 19 Dec 2025 16:44:11 -0800
>Deepak Gupta <debug@rivosinc.com> wrote:
>
>> On Thu, Dec 18, 2025 at 08:13:31PM +0100, Lukas Gerlach wrote:
>> >Similarly to x86 and arm64, mitigate speculation past an access_ok()
>> >check by masking the pointer before use.
>> >
>> >On RISC-V, user addresses have the MSB clear while kernel addresses
>> >have the MSB set. The uaccess_mask_ptr() function clears the MSB,
>> >ensuring any kernel pointer becomes invalid and will fault, while
>> >valid user pointers remain unchanged. This prevents speculative
>> >access to kernel memory via user copy functions.
>> >
>> >The masking is applied to __get_user, __put_user, raw_copy_from_user,
>> >raw_copy_to_user, clear_user, and the unsafe_* variants.
>> >
>> >Signed-off-by: Lukas Gerlach <lukas.gerlach@cispa.de>
>> >---
>> > arch/riscv/include/asm/uaccess.h | 41 +++++++++++++++++++++++++-------
>> > 1 file changed, 32 insertions(+), 9 deletions(-)
>> >
>> >diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
>> >index 36bba6720c26..ceee1d62ff9b 100644
>> >--- a/arch/riscv/include/asm/uaccess.h
>> >+++ b/arch/riscv/include/asm/uaccess.h
>> >@@ -74,6 +74,23 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm, unsigne
>> > #define __typefits(x, type, not) \
>> > 	__builtin_choose_expr(sizeof(x) <= sizeof(type), (unsigned type)0, not)
>> >
>> >+/*
>> >+ * Sanitize a uaccess pointer such that it cannot reach any kernel address.
>> >+ *
>> >+ * On RISC-V, virtual addresses are sign-extended from the top implemented bit.
>> >+ * User addresses have the MSB clear; kernel addresses have the MSB set.
>> >+ * Clearing the MSB ensures any kernel pointer becomes non-canonical and will
>> >+ * fault, while valid user pointers remain unchanged.
>> >+ */
>> >+#define uaccess_mask_ptr(ptr) ((__typeof__(ptr))__uaccess_mask_ptr(ptr))
>> >+static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
>> >+{
>> >+	unsigned long val = (unsigned long)ptr;
>> >+
>> >+	val = (val << 1) >> 1;
>> >+	return (void __user *)val;
>>
>> This is only clearing b63 which is what we don't need here.
>
>It is also entirely the wrong operation.
>A kernel address needs converting into an address that is guaranteed
>to fault - not a user address that might be valid.

This is about speculative accesses and not actual accesses. Due to some
speculation it is possible that in speculative path a wrong address is
generated with MSB=1. This simply ensures that bit is cleared for agen
even in speculative path.

>
>You need a guard page of address space between valid user addresses
>and valid kernel addresses that is guaranteed to not be used.

IIUC, risc-v does that already (last user page is unmapped and first
kernel page is unmapped).

>Typically this is the top page of the user address space.
>All kernel addresses need converting to the base of the guard page
>using ALU operations (to avoid speculative accesses).
>So: ptr = min(ptr, guard_page); easiest done (as in x86-64) with
>a compare and conditional move.
>The ppc version is more complex because there isn't a usable conditional
>move instruction.
>I think this would work for x86-32:
>	offset = ptr + -guard_page;
>	mask = sbb(x,x); // ~0u if the add set the carry flag.
>	ptr -= offset & mask;
>You need to find something that works for riscv.

I believe what you're describing is `access_ok` in x86. `__access_ok` in
`asm-generic/access_ok.h` should cover same behavior for risc-v.

>
>>
>> You should be clearing b47 (if bit indexing starts at 0) on Sv48 and b56
>> on Sv57 system.
>>
>> Anything above b47/b56 isn't going to be used anyways in indexing into
>> page tables and will be ignored if pointer masking is enabled at S.
>
>Gah more broken hardware...
>Ignoring the high address bits doesn't work and is really a bad idea.
>Arm did it as well.
>Trying to do 'user pointer masking' for uaccess validation is a PITA
>if you have to allow for non-zero values in the high bits it makes
>life even more complicated.

Pointer masking and preventing speculative accesses to kernel addresses
are two different things (although they're related because they impact
address generation part). 

Kernel may not have pointer masking enabled and in that case, it'll just
trap if high unused bits don't match b38/47/56. To accomodate that, there
already are patches in riscv to remove the tag in high unused bits before
access is made by the kernel.

>
>And 'pointer masking' is so broken unless it is done at the page level
>and enforced by the hardware.
>What it does is make random/invalid addresses much more likely to be valid.
>An interpreter can use them internally, but they are so slow software
>masking  (following the validation) shouldn't be a real issue.
>
>	David
>
>
Re: [PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation
Posted by David Laight 1 month, 1 week ago
On Sat, 27 Dec 2025 17:59:38 -0800
Deepak Gupta <debug@rivosinc.com> wrote:

> On Sat, Dec 27, 2025 at 09:28:59PM +0000, David Laight wrote:
> >On Fri, 19 Dec 2025 16:44:11 -0800
> >Deepak Gupta <debug@rivosinc.com> wrote:
> >  
> >> On Thu, Dec 18, 2025 at 08:13:31PM +0100, Lukas Gerlach wrote:  
> >> >Similarly to x86 and arm64, mitigate speculation past an access_ok()
> >> >check by masking the pointer before use.
> >> >
> >> >On RISC-V, user addresses have the MSB clear while kernel addresses
> >> >have the MSB set. The uaccess_mask_ptr() function clears the MSB,
> >> >ensuring any kernel pointer becomes invalid and will fault, while
> >> >valid user pointers remain unchanged. This prevents speculative
> >> >access to kernel memory via user copy functions.
> >> >
> >> >The masking is applied to __get_user, __put_user, raw_copy_from_user,
> >> >raw_copy_to_user, clear_user, and the unsafe_* variants.
> >> >
> >> >Signed-off-by: Lukas Gerlach <lukas.gerlach@cispa.de>
> >> >---
> >> > arch/riscv/include/asm/uaccess.h | 41 +++++++++++++++++++++++++-------
> >> > 1 file changed, 32 insertions(+), 9 deletions(-)
> >> >
> >> >diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
> >> >index 36bba6720c26..ceee1d62ff9b 100644
> >> >--- a/arch/riscv/include/asm/uaccess.h
> >> >+++ b/arch/riscv/include/asm/uaccess.h
> >> >@@ -74,6 +74,23 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm, unsigne
> >> > #define __typefits(x, type, not) \
> >> > 	__builtin_choose_expr(sizeof(x) <= sizeof(type), (unsigned type)0, not)
> >> >
> >> >+/*
> >> >+ * Sanitize a uaccess pointer such that it cannot reach any kernel address.
> >> >+ *
> >> >+ * On RISC-V, virtual addresses are sign-extended from the top implemented bit.
> >> >+ * User addresses have the MSB clear; kernel addresses have the MSB set.
> >> >+ * Clearing the MSB ensures any kernel pointer becomes non-canonical and will
> >> >+ * fault, while valid user pointers remain unchanged.
> >> >+ */
> >> >+#define uaccess_mask_ptr(ptr) ((__typeof__(ptr))__uaccess_mask_ptr(ptr))
> >> >+static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
> >> >+{
> >> >+	unsigned long val = (unsigned long)ptr;
> >> >+
> >> >+	val = (val << 1) >> 1;
> >> >+	return (void __user *)val;  
> >>
> >> This is only clearing b63 which is what we don't need here.  
> >
> >It is also entirely the wrong operation.
> >A kernel address needs converting into an address that is guaranteed
> >to fault - not a user address that might be valid.  
> 
> This is about speculative accesses and not actual accesses. Due to some
> speculation it is possible that in speculative path a wrong address is
> generated with MSB=1. This simply ensures that bit is cleared for agen
> even in speculative path.

You said you were following what x86 did - this isn't what is does.
Avoiding the conditional branch in access_ok() is actually a big win.
That is true even without the issues with speculative accesses to kernel
memory.
The 'address masking' (badly named - it isn't just masking) is a replacement
for access_ok(), it changes kernel addresses to invalid ones so that the
access faults and the trap/fault fixup code detects the error.

	David

> 
> >
> >You need a guard page of address space between valid user addresses
> >and valid kernel addresses that is guaranteed to not be used.  
> 
> IIUC, risc-v does that already (last user page is unmapped and first
> kernel page is unmapped).
> 
> >Typically this is the top page of the user address space.
> >All kernel addresses need converting to the base of the guard page
> >using ALU operations (to avoid speculative accesses).
> >So: ptr = min(ptr, guard_page); easiest done (as in x86-64) with
> >a compare and conditional move.
> >The ppc version is more complex because there isn't a usable conditional
> >move instruction.
> >I think this would work for x86-32:
> >	offset = ptr + -guard_page;
> >	mask = sbb(x,x); // ~0u if the add set the carry flag.
> >	ptr -= offset & mask;
> >You need to find something that works for riscv.  
> 
> I believe what you're describing is `access_ok` in x86. `__access_ok` in
> `asm-generic/access_ok.h` should cover same behavior for risc-v.
> 
> >  
> >>
> >> You should be clearing b47 (if bit indexing starts at 0) on Sv48 and b56
> >> on Sv57 system.
> >>
> >> Anything above b47/b56 isn't going to be used anyways in indexing into
> >> page tables and will be ignored if pointer masking is enabled at S.  
> >
> >Gah more broken hardware...
> >Ignoring the high address bits doesn't work and is really a bad idea.
> >Arm did it as well.
> >Trying to do 'user pointer masking' for uaccess validation is a PITA
> >if you have to allow for non-zero values in the high bits it makes
> >life even more complicated.  
> 
> Pointer masking and preventing speculative accesses to kernel addresses
> are two different things (although they're related because they impact
> address generation part). 
> 
> Kernel may not have pointer masking enabled and in that case, it'll just
> trap if high unused bits don't match b38/47/56. To accomodate that, there
> already are patches in riscv to remove the tag in high unused bits before
> access is made by the kernel.
> 
> >
> >And 'pointer masking' is so broken unless it is done at the page level
> >and enforced by the hardware.
> >What it does is make random/invalid addresses much more likely to be valid.
> >An interpreter can use them internally, but they are so slow software
> >masking  (following the validation) shouldn't be a real issue.
> >
> >	David
> >
> >
Re: [PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation
Posted by David Laight 1 month, 1 week ago
On Sun, 28 Dec 2025 22:34:30 +0000
David Laight <david.laight.linux@gmail.com> wrote:

> On Sat, 27 Dec 2025 17:59:38 -0800
> Deepak Gupta <debug@rivosinc.com> wrote:
> 
> > On Sat, Dec 27, 2025 at 09:28:59PM +0000, David Laight wrote:  
> > >On Fri, 19 Dec 2025 16:44:11 -0800
> > >Deepak Gupta <debug@rivosinc.com> wrote:
> > >    
> > >> On Thu, Dec 18, 2025 at 08:13:31PM +0100, Lukas Gerlach wrote:    
> > >> >Similarly to x86 and arm64, mitigate speculation past an access_ok()
> > >> >check by masking the pointer before use.
> > >> >
> > >> >On RISC-V, user addresses have the MSB clear while kernel addresses
> > >> >have the MSB set. The uaccess_mask_ptr() function clears the MSB,
> > >> >ensuring any kernel pointer becomes invalid and will fault, while
> > >> >valid user pointers remain unchanged. This prevents speculative
> > >> >access to kernel memory via user copy functions.
> > >> >
> > >> >The masking is applied to __get_user, __put_user, raw_copy_from_user,
> > >> >raw_copy_to_user, clear_user, and the unsafe_* variants.
> > >> >
> > >> >Signed-off-by: Lukas Gerlach <lukas.gerlach@cispa.de>
> > >> >---
> > >> > arch/riscv/include/asm/uaccess.h | 41 +++++++++++++++++++++++++-------
> > >> > 1 file changed, 32 insertions(+), 9 deletions(-)
> > >> >
> > >> >diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
> > >> >index 36bba6720c26..ceee1d62ff9b 100644
> > >> >--- a/arch/riscv/include/asm/uaccess.h
> > >> >+++ b/arch/riscv/include/asm/uaccess.h
> > >> >@@ -74,6 +74,23 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm, unsigne
> > >> > #define __typefits(x, type, not) \
> > >> > 	__builtin_choose_expr(sizeof(x) <= sizeof(type), (unsigned type)0, not)
> > >> >
> > >> >+/*
> > >> >+ * Sanitize a uaccess pointer such that it cannot reach any kernel address.
> > >> >+ *
> > >> >+ * On RISC-V, virtual addresses are sign-extended from the top implemented bit.
> > >> >+ * User addresses have the MSB clear; kernel addresses have the MSB set.
> > >> >+ * Clearing the MSB ensures any kernel pointer becomes non-canonical and will
> > >> >+ * fault, while valid user pointers remain unchanged.
> > >> >+ */
> > >> >+#define uaccess_mask_ptr(ptr) ((__typeof__(ptr))__uaccess_mask_ptr(ptr))
> > >> >+static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
> > >> >+{
> > >> >+	unsigned long val = (unsigned long)ptr;
> > >> >+
> > >> >+	val = (val << 1) >> 1;
> > >> >+	return (void __user *)val;    
> > >>
> > >> This is only clearing b63 which is what we don't need here.    
> > >
> > >It is also entirely the wrong operation.
> > >A kernel address needs converting into an address that is guaranteed
> > >to fault - not a user address that might be valid.    
> > 
> > This is about speculative accesses and not actual accesses. Due to some
> > speculation it is possible that in speculative path a wrong address is
> > generated with MSB=1. This simply ensures that bit is cleared for agen
> > even in speculative path.  
> 
> You said you were following what x86 did - this isn't what is does.
> Avoiding the conditional branch in access_ok() is actually a big win.
> That is true even without the issues with speculative accesses to kernel
> memory.
> The 'address masking' (badly named - it isn't just masking) is a replacement
> for access_ok(), it changes kernel addresses to invalid ones so that the
> access faults and the trap/fault fixup code detects the error.

If you can guarantee that address 0 is never mapped into userspace
(not true for x86 for historic reasons) then I think you can convert
kernel addresses to zero - which will then fault.
This is simpler than using the base of the guard page (which is still
needed for sequential accesses).
Something like:
	addr &= (addr >= guard_page) - 1;
will then work - but it needs (partially) coding in assembler to
guarantee that 'setae' (or equivalent) is used rather than a
conditional branch.
 
	David
Re: [PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation
Posted by Vivian Wang 1 month, 1 week ago
On 12/29/25 20:32, David Laight wrote:
> On Sun, 28 Dec 2025 22:34:30 +0000
> David Laight <david.laight.linux@gmail.com> wrote:
>
>> On Sat, 27 Dec 2025 17:59:38 -0800
>> Deepak Gupta <debug@rivosinc.com> wrote:
>>
>>> On Sat, Dec 27, 2025 at 09:28:59PM +0000, David Laight wrote:  
>>>> On Fri, 19 Dec 2025 16:44:11 -0800
>>>> Deepak Gupta <debug@rivosinc.com> wrote:
>>>>    
>>>>> On Thu, Dec 18, 2025 at 08:13:31PM +0100, Lukas Gerlach wrote:    
>>>>>> Similarly to x86 and arm64, mitigate speculation past an access_ok()
>>>>>> check by masking the pointer before use.
>>>>>>
>>>>>> On RISC-V, user addresses have the MSB clear while kernel addresses
>>>>>> have the MSB set. The uaccess_mask_ptr() function clears the MSB,
>>>>>> ensuring any kernel pointer becomes invalid and will fault, while
>>>>>> valid user pointers remain unchanged. This prevents speculative
>>>>>> access to kernel memory via user copy functions.
>>>>>>
>>>>>> The masking is applied to __get_user, __put_user, raw_copy_from_user,
>>>>>> raw_copy_to_user, clear_user, and the unsafe_* variants.
>>>>>>
>>>>>> Signed-off-by: Lukas Gerlach <lukas.gerlach@cispa.de>
>>>>>> ---
>>>>>> arch/riscv/include/asm/uaccess.h | 41 +++++++++++++++++++++++++-------
>>>>>> 1 file changed, 32 insertions(+), 9 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
>>>>>> index 36bba6720c26..ceee1d62ff9b 100644
>>>>>> --- a/arch/riscv/include/asm/uaccess.h
>>>>>> +++ b/arch/riscv/include/asm/uaccess.h
>>>>>> @@ -74,6 +74,23 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm, unsigne
>>>>>> #define __typefits(x, type, not) \
>>>>>> 	__builtin_choose_expr(sizeof(x) <= sizeof(type), (unsigned type)0, not)
>>>>>>
>>>>>> +/*
>>>>>> + * Sanitize a uaccess pointer such that it cannot reach any kernel address.
>>>>>> + *
>>>>>> + * On RISC-V, virtual addresses are sign-extended from the top implemented bit.
>>>>>> + * User addresses have the MSB clear; kernel addresses have the MSB set.
>>>>>> + * Clearing the MSB ensures any kernel pointer becomes non-canonical and will
>>>>>> + * fault, while valid user pointers remain unchanged.
>>>>>> + */
>>>>>> +#define uaccess_mask_ptr(ptr) ((__typeof__(ptr))__uaccess_mask_ptr(ptr))
>>>>>> +static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
>>>>>> +{
>>>>>> +	unsigned long val = (unsigned long)ptr;
>>>>>> +
>>>>>> +	val = (val << 1) >> 1;
>>>>>> +	return (void __user *)val;    
>>>>> This is only clearing b63 which is what we don't need here.    
>>>> It is also entirely the wrong operation.
>>>> A kernel address needs converting into an address that is guaranteed
>>>> to fault - not a user address that might be valid.    
>>> This is about speculative accesses and not actual accesses. Due to some
>>> speculation it is possible that in speculative path a wrong address is
>>> generated with MSB=1. This simply ensures that bit is cleared for agen
>>> even in speculative path.  
>> You said you were following what x86 did - this isn't what is does.
>> Avoiding the conditional branch in access_ok() is actually a big win.
>> That is true even without the issues with speculative accesses to kernel
>> memory.
>> The 'address masking' (badly named - it isn't just masking) is a replacement
>> for access_ok(), it changes kernel addresses to invalid ones so that the
>> access faults and the trap/fault fixup code detects the error.
> If you can guarantee that address 0 is never mapped into userspace
> (not true for x86 for historic reasons) then I think you can convert
> kernel addresses to zero - which will then fault.
> This is simpler than using the base of the guard page (which is still
> needed for sequential accesses).
> Something like:
> 	addr &= (addr >= guard_page) - 1;
> will then work - but it needs (partially) coding in assembler to
> guarantee that 'setae' (or equivalent) is used rather than a
> conditional branch.
>  

Maybe use:

    addr |= (long)addr >> 63

To map kernel addresses to -1?
Re: [PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation
Posted by David Laight 1 month, 1 week ago
On Wed, 31 Dec 2025 11:47:16 +0800
Vivian Wang <wangruikang@iscas.ac.cn> wrote:

> On 12/29/25 20:32, David Laight wrote:
> > On Sun, 28 Dec 2025 22:34:30 +0000
> > David Laight <david.laight.linux@gmail.com> wrote:
> >  
> >> On Sat, 27 Dec 2025 17:59:38 -0800
> >> Deepak Gupta <debug@rivosinc.com> wrote:
> >>  
> >>> On Sat, Dec 27, 2025 at 09:28:59PM +0000, David Laight wrote:    
> >>>> On Fri, 19 Dec 2025 16:44:11 -0800
> >>>> Deepak Gupta <debug@rivosinc.com> wrote:
> >>>>      
> >>>>> On Thu, Dec 18, 2025 at 08:13:31PM +0100, Lukas Gerlach wrote:      
> >>>>>> Similarly to x86 and arm64, mitigate speculation past an access_ok()
> >>>>>> check by masking the pointer before use.
> >>>>>>
> >>>>>> On RISC-V, user addresses have the MSB clear while kernel addresses
> >>>>>> have the MSB set. The uaccess_mask_ptr() function clears the MSB,
> >>>>>> ensuring any kernel pointer becomes invalid and will fault, while
> >>>>>> valid user pointers remain unchanged. This prevents speculative
> >>>>>> access to kernel memory via user copy functions.
> >>>>>>
> >>>>>> The masking is applied to __get_user, __put_user, raw_copy_from_user,
> >>>>>> raw_copy_to_user, clear_user, and the unsafe_* variants.
> >>>>>>
> >>>>>> Signed-off-by: Lukas Gerlach <lukas.gerlach@cispa.de>
> >>>>>> ---
> >>>>>> arch/riscv/include/asm/uaccess.h | 41 +++++++++++++++++++++++++-------
> >>>>>> 1 file changed, 32 insertions(+), 9 deletions(-)
> >>>>>>
> >>>>>> diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
> >>>>>> index 36bba6720c26..ceee1d62ff9b 100644
> >>>>>> --- a/arch/riscv/include/asm/uaccess.h
> >>>>>> +++ b/arch/riscv/include/asm/uaccess.h
> >>>>>> @@ -74,6 +74,23 @@ static inline unsigned long __untagged_addr_remote(struct mm_struct *mm, unsigne
> >>>>>> #define __typefits(x, type, not) \
> >>>>>> 	__builtin_choose_expr(sizeof(x) <= sizeof(type), (unsigned type)0, not)
> >>>>>>
> >>>>>> +/*
> >>>>>> + * Sanitize a uaccess pointer such that it cannot reach any kernel address.
> >>>>>> + *
> >>>>>> + * On RISC-V, virtual addresses are sign-extended from the top implemented bit.
> >>>>>> + * User addresses have the MSB clear; kernel addresses have the MSB set.
> >>>>>> + * Clearing the MSB ensures any kernel pointer becomes non-canonical and will
> >>>>>> + * fault, while valid user pointers remain unchanged.
> >>>>>> + */
> >>>>>> +#define uaccess_mask_ptr(ptr) ((__typeof__(ptr))__uaccess_mask_ptr(ptr))
> >>>>>> +static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
> >>>>>> +{
> >>>>>> +	unsigned long val = (unsigned long)ptr;
> >>>>>> +
> >>>>>> +	val = (val << 1) >> 1;
> >>>>>> +	return (void __user *)val;      
> >>>>> This is only clearing b63 which is what we don't need here.      
> >>>> It is also entirely the wrong operation.
> >>>> A kernel address needs converting into an address that is guaranteed
> >>>> to fault - not a user address that might be valid.      
> >>> This is about speculative accesses and not actual accesses. Due to some
> >>> speculation it is possible that in speculative path a wrong address is
> >>> generated with MSB=1. This simply ensures that bit is cleared for agen
> >>> even in speculative path.    
> >> You said you were following what x86 did - this isn't what is does.
> >> Avoiding the conditional branch in access_ok() is actually a big win.
> >> That is true even without the issues with speculative accesses to kernel
> >> memory.
> >> The 'address masking' (badly named - it isn't just masking) is a replacement
> >> for access_ok(), it changes kernel addresses to invalid ones so that the
> >> access faults and the trap/fault fixup code detects the error.  
> > If you can guarantee that address 0 is never mapped into userspace
> > (not true for x86 for historic reasons) then I think you can convert
> > kernel addresses to zero - which will then fault.
> > This is simpler than using the base of the guard page (which is still
> > needed for sequential accesses).
> > Something like:
> > 	addr &= (addr >= guard_page) - 1;
> > will then work - but it needs (partially) coding in assembler to
> > guarantee that 'setae' (or equivalent) is used rather than a
> > conditional branch.
> >    
> 
> Maybe use:
> 
>     addr |= (long)addr >> 63
> 
> To map kernel addresses to -1?

That also requires that address 0 is never mapped into userspace.
The first access might be offset from the actual base address.
(It is too hard to check that code accesses the first member of a
structure first - and some obvious candidate bits of code don't.)

There are also issues on x86-64 (I think amd cpu) which mean that
just testing the high bit isn't good enough, the check has to use
the highest valid user address (ie one below the 'big address hole').
Linus added to code to patch the correct constant into the comparison
instruction in order to avoid a memory read because a different limit
is needed to 4 and 5 level page tables. 

Whether that applies to riscv is another matter, but if the hardware
actually uses a lower bit to select kernel addresses (perhaps in
parallel with validating that the top few bits are all the same)
then you do need to compare against the top user address. 

I suspect that if you don't do it now, at some point there'll be
an 'issue' that needs the correct limit address used.

	David
Re: [PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation
Posted by Lukas Gerlach 1 month, 1 week ago
Thanks for the review. You're right - we should only clear the sign bit
(b38/b47/b56 depending on mode), not b63. Clearing upper bits would
interfere with pointer masking.

Here's a fix that computes the sign bit position arithmetically to avoid
branches, this ensures the mitigation cannot be bypassed under speculation.
This is basically the VA_BITS macro but computed in a branch-free way.

In arch/riscv/include/asm/uaccess.h:

  #define UACCESS_SIGN_BIT \
      (VA_BITS_SV39 - 1 + 9*((unsigned long)pgtable_l4_enabled) + \
       9*((unsigned long)pgtable_l5_enabled))

  #define uaccess_mask_ptr(ptr) ((__typeof__(ptr))__uaccess_mask_ptr(ptr))
  static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
  {
      return (void __user *)((unsigned long)ptr & ~BIT_ULL(UACCESS_SIGN_BIT));
  }

This evaluates to bit 38 for Sv39, bit 47 for Sv48, and bit 56 for Sv57.
Re: [PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation
Posted by Deepak Gupta 1 month, 1 week ago
On Sat, Dec 27, 2025 at 01:57:03PM +0100, Lukas Gerlach wrote:
>Thanks for the review. You're right - we should only clear the sign bit
>(b38/b47/b56 depending on mode), not b63. Clearing upper bits would
>interfere with pointer masking.
>
>Here's a fix that computes the sign bit position arithmetically to avoid
>branches, this ensures the mitigation cannot be bypassed under speculation.
>This is basically the VA_BITS macro but computed in a branch-free way.
>
>In arch/riscv/include/asm/uaccess.h:
>
>  #define UACCESS_SIGN_BIT \
>      (VA_BITS_SV39 - 1 + 9*((unsigned long)pgtable_l4_enabled) + \
>       9*((unsigned long)pgtable_l5_enabled))
>
>  #define uaccess_mask_ptr(ptr) ((__typeof__(ptr))__uaccess_mask_ptr(ptr))
>  static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
>  {
>      return (void __user *)((unsigned long)ptr & ~BIT_ULL(UACCESS_SIGN_BIT));
>  }
>
>This evaluates to bit 38 for Sv39, bit 47 for Sv48, and bit 56 for Sv57.

looks good to me.
Although, I am concerned about maintainibility and bit-rotting.
I would suggest to fix VA_BITS definition instead of defining a new macro here.