Linux-RISC-V Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Deepak Gupta <debug@rivosinc.com>
Cc: Lukas Gerlach <lukas.gerlach@cispa.de>,
	linux-riscv@lists.infradead.org, palmer@dabbelt.com,
	pjw@kernel.org, aou@eecs.berkeley.edu, alex@ghiti.fr,
	linux-kernel@vger.kernel.org, daniel.weber@cispa.de,
	michael.schwarz@cispa.de, marton.bognar@kuleuven.be,
	jo.vanbulck@kuleuven.be
Subject: Re: [PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation
Date: Mon, 29 Dec 2025 12:32:12 +0000	[thread overview]
Message-ID: <20251229123212.25ef3c4b@pumpkin> (raw)
In-Reply-To: <20251228223430.43f14d51@pumpkin>

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2025-12-29 12:32 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-18 19:13 [PATCH 0/2] riscv: Add Spectre v1 mitigations Lukas Gerlach
2025-12-18 19:13 ` [PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation Lukas Gerlach
2025-12-20  0:44   ` Deepak Gupta
2025-12-27 12:57     ` Lukas Gerlach
2025-12-28  0:41       ` Deepak Gupta
2025-12-27 21:28     ` David Laight
2025-12-28  1:59       ` Deepak Gupta
2025-12-28 22:34         ` David Laight
2025-12-29 12:32           ` David Laight [this message]
2025-12-31  3:47             ` Vivian Wang
2025-12-31 10:35               ` David Laight
2025-12-18 19:13 ` [PATCH 2/2] riscv: Sanitize syscall table indexing under speculation Lukas Gerlach
2025-12-31  3:01   ` Paul Walmsley
2025-12-31  3:31 ` [PATCH 0/2] riscv: Add Spectre v1 mitigations patchwork-bot+linux-riscv
2026-01-05 23:17   ` Paul Walmsley
2026-01-06 10:30     ` [PATCH 1/2] riscv: Use pointer masking to limit uaccess speculation Lukas Gerlach

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251229123212.25ef3c4b@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=alex@ghiti.fr \
    --cc=aou@eecs.berkeley.edu \
    --cc=daniel.weber@cispa.de \
    --cc=debug@rivosinc.com \
    --cc=jo.vanbulck@kuleuven.be \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=lukas.gerlach@cispa.de \
    --cc=marton.bognar@kuleuven.be \
    --cc=michael.schwarz@cispa.de \
    --cc=palmer@dabbelt.com \
    --cc=pjw@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox