[PATCH] riscv: mm: Implement arch_within_stack_frames() for HARDENED_USERCOPY

[PATCH] riscv: mm: Implement arch_within_stack_frames() for HARDENED_USERCOPY

[cp0613]

From: Chen Pei <cp0613@linux.alibaba.com>

Implement arch_within_stack_frames() to enable precise per-frame stack
object validation for CONFIG_HARDENED_USERCOPY on RISC-V.

== Background ==

CONFIG_HARDENED_USERCOPY validates kernel objects passed to copy_to/
from_user(). For stack objects, mm/usercopy.c calls arch_within_stack_
frames() to determine whether the object lies within a valid stack
frame's local variable area. Without an arch-specific implementation,
the generic fallback returns NOT_STACK (0), causing check_stack_object()
to skip frame-level validation and fall through to a coarser depth
check only.

== RISC-V Stack Frame Layout ==

With -fno-omit-frame-pointer (implied by CONFIG_FRAME_POINTER), the
RISC-V ABI places the saved frame pointer (fp/s0) and return address
(ra) at the top of each frame:

  high addr
    +------------------+  <--- fp (s0) -- frame pointer register
    |   saved ra       |      fp - 8  (return address of this function)
    |   saved fp       |      fp - 16 (caller's frame pointer)
    +------------------+
    |   local variables|
    |   spilled args   |
    +------------------+  <--- sp
  low addr

The kernel's struct stackframe { fp; ra } (asm/stacktrace.h) lives at
(fp - sizeof(stackframe)), i.e., saved_fp at fp-16 and saved_ra at
fp-8. Walking the chain: next_fp = *(fp - 16).

This differs from x86, where [saved_bp][saved_ip] are at the frame
bottom and the local area is above them.

== Implementation ==

The allowed usercopy region within one frame is:

  [prev_fp, fp - 2*sizeof(void*))

i.e., from the base of the current frame's locals (prev_fp) up to (but
not including) the saved fp/ra area at the top of the frame.

  - obj entirely within this region -> GOOD_FRAME
  - obj overlapping saved fp/ra or outside any frame -> BAD_STACK
  - CONFIG_FRAME_POINTER not set -> NOT_STACK

The frame chain is walked starting from __builtin_frame_address(0),
with prev_fp initialized to current_stack_pointer (not the thread's
stack base). This ensures objects in already-returned frames (address
below current sp) are correctly detected as BAD_STACK, because no live
frame in the chain will claim that region.

The function is marked __no_kmsan_checks because it reads from another
frame's stack memory, whose KMSAN shadow may not be set up, which would
cause false positive KMSAN reports.

The entire implementation is guarded by #ifndef __ASSEMBLER__ since
this header is included by both C and assembly files, and attributes
like __no_kmsan_checks are not valid in assembler context.

Select HAVE_ARCH_WITHIN_STACK_FRAMES in Kconfig to replace the generic
no-op with the RISC-V implementation.

== Testing ==

Build requirements:
  CONFIG_HARDENED_USERCOPY=y
  CONFIG_FRAME_POINTER=y      (already selected by default on riscv)
  CONFIG_LKDTM=y
  CONFIG_DEBUG_FS=y

Run on QEMU or target board as root:

  # Trigger copy_to_user from a stack frame that has already returned:
  echo USERCOPY_STACK_FRAME_TO > /sys/kernel/debug/provoke-crash/DIRECT

  # Trigger copy_from_user into a stack frame that has already returned:
  echo USERCOPY_STACK_FRAME_FROM > /sys/kernel/debug/provoke-crash/DIRECT

  # Trigger copy beyond current stack bounds:
  echo USERCOPY_STACK_BEYOND > /sys/kernel/debug/provoke-crash/DIRECT

== Kernel test logs ==

  # echo USERCOPY_STACK_FRAME_TO > /sys/kernel/debug/provoke-crash/DIRECT
  lkdtm: Performing direct entry USERCOPY_STACK_FRAME_TO
  lkdtm: good_stack: ff20000000323c98-ff20000000323cb8
  lkdtm: bad_stack : ff20000000323c18-ff20000000323c38
  lkdtm: attempting good copy_to_user of local stack
  lkdtm: attempting bad copy_to_user of distant stack
  usercopy: Kernel memory exposure attempt detected from process stack (offset 38, size 20)!
  ......
  (kernel BUG Call Trace...)
  ......

Without this patch (generic fallback returns NOT_STACK), the "bad
copy_to_user" call above would succeed silently. With this patch,
HARDENED_USERCOPY correctly detects the cross-frame violation and
aborts with BUG().

Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
---
 arch/riscv/Kconfig                   |  1 +
 arch/riscv/include/asm/thread_info.h | 66 ++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 6fe90591a274..4f765ff608d9 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -150,6 +150,7 @@ config RISCV
 	select HAVE_ARCH_USERFAULTFD_MINOR if 64BIT && USERFAULTFD
 	select HAVE_ARCH_USERFAULTFD_WP if 64BIT && MMU && USERFAULTFD && RISCV_ISA_SVRSW60T59B
 	select HAVE_ARCH_VMAP_STACK if MMU && 64BIT
+	select HAVE_ARCH_WITHIN_STACK_FRAMES
 	select HAVE_ASM_MODVERSIONS
 	select HAVE_BUILDTIME_MCOUNT_SORT
 	select HAVE_CONTEXT_TRACKING_USER
diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index 36918c9200c9..616a41eb7416 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -101,6 +101,72 @@ struct thread_info {
 void arch_release_task_struct(struct task_struct *tsk);
 int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
 
+#ifdef CONFIG_HAVE_ARCH_WITHIN_STACK_FRAMES
+/*
+ * RISC-V stack frame layout (with frame pointer enabled):
+ *
+ * high addr
+ *   +------------------+  <--- fp (s0) points here
+ *   |   saved ra       |  fp - 8  (return address)
+ *   |   saved fp       |  fp - 16 (previous frame pointer)
+ *   +------------------+
+ *   |   local vars     |
+ *   |   arguments      |
+ *   +------------------+  <--- sp
+ * low addr
+ *
+ * The struct stackframe { fp, ra } lives at (fp - sizeof(stackframe)),
+ * i.e. fp[-2]=saved_fp and fp[-1]=saved_ra.
+ *
+ * For usercopy safety, we allow copies within [prev_fp, fp - 2*sizeof(void*))
+ * for each frame in the chain, where prev_fp is the fp of the previous
+ * (lower) frame.  This covers local variables and arguments but excludes
+ * the saved ra/fp slots at the top of the frame.
+ *
+ * We walk the frame chain starting from __builtin_frame_address(0) (the
+ * current frame), with prev_fp initialized to current_stack_pointer.
+ * Using current_stack_pointer -- rather than the 'stack' argument (which is
+ * the thread's entire stack base) -- ensures that objects in already-returned
+ * frames (address below current sp) are correctly detected as BAD_STACK,
+ * because no live frame in the chain will claim that region.
+ */
+__no_kmsan_checks
+static inline int arch_within_stack_frames(const void * const stack,
+					   const void * const stackend,
+					   const void *obj, unsigned long len)
+{
+#if defined(CONFIG_FRAME_POINTER)
+	const void *fp = (const void *)__builtin_frame_address(0);
+	const void *prev_fp = (const void *)current_stack_pointer;
+
+	/*
+	 * Walk the frame chain. Each iteration checks whether [obj, obj+len)
+	 * falls within the local-variable area of the current frame:
+	 *
+	 *   [prev_fp, fp - 2*sizeof(void*))
+	 *
+	 * i.e. from the base of this frame (sp of this frame, which equals
+	 * the fp of the frame below) up to (but not including) the saved
+	 * fp/ra area at the top of this frame.
+	 */
+	while (stack <= fp && fp < stackend) {
+		const void *frame_vars_end = (const char *)fp - 2 * sizeof(void *);
+
+		if (obj + len <= frame_vars_end) {
+			if (obj >= prev_fp)
+				return GOOD_FRAME;
+			return BAD_STACK;
+		}
+		prev_fp = fp;
+		fp = *(const void * const *)((const char *)fp - 2 * sizeof(void *));
+	}
+	return BAD_STACK;
+#else
+	return NOT_STACK;
+#endif
+}
+#endif /* CONFIG_HAVE_ARCH_WITHIN_STACK_FRAMES */
+
 #endif /* !__ASSEMBLER__ */
 
 /*
-- 
2.50.1


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

Re: [PATCH] riscv: mm: Implement arch_within_stack_frames() for HARDENED_USERCOPY

[Vivian Wang]

Hi Chen Pei,

Thanks for your patch. I have a few minor suggestions:

On 4/10/26 09:50, cp0613@linux.alibaba.com wrote:
> From: Chen Pei <cp0613@linux.alibaba.com>
>
> Implement arch_within_stack_frames() to enable precise per-frame stack
> object validation for CONFIG_HARDENED_USERCOPY on RISC-V.
>
> == Background ==
>
> [...]
>
> Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>

Patch message is probably too long. You can put additional information
like testing under a --- line to cut those out of the eventual actual
commit message.

Make sure that Signed-off-by is above the cut if you do that.

> ---
>  arch/riscv/Kconfig                   |  1 +
>  arch/riscv/include/asm/thread_info.h | 66 ++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 6fe90591a274..4f765ff608d9 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -150,6 +150,7 @@ config RISCV
>  	select HAVE_ARCH_USERFAULTFD_MINOR if 64BIT && USERFAULTFD
>  	select HAVE_ARCH_USERFAULTFD_WP if 64BIT && MMU && USERFAULTFD && RISCV_ISA_SVRSW60T59B
>  	select HAVE_ARCH_VMAP_STACK if MMU && 64BIT
> +	select HAVE_ARCH_WITHIN_STACK_FRAMES
>  	select HAVE_ASM_MODVERSIONS
>  	select HAVE_BUILDTIME_MCOUNT_SORT
>  	select HAVE_CONTEXT_TRACKING_USER
> diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> index 36918c9200c9..616a41eb7416 100644
> --- a/arch/riscv/include/asm/thread_info.h
> +++ b/arch/riscv/include/asm/thread_info.h
> @@ -101,6 +101,72 @@ struct thread_info {
>  void arch_release_task_struct(struct task_struct *tsk);
>  int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
>  
> +#ifdef CONFIG_HAVE_ARCH_WITHIN_STACK_FRAMES

Not needed since you've selected it above.

> +/*
> + * RISC-V stack frame layout (with frame pointer enabled):
> + *
> + * high addr
> + *   +------------------+  <--- fp (s0) points here
> + *   |   saved ra       |  fp - 8  (return address)
> + *   |   saved fp       |  fp - 16 (previous frame pointer)

fp - 2*sizeof(void*) probably, to account for RV32?

> + *   +------------------+
> + *   |   local vars     |
> + *   |   arguments      |
> + *   +------------------+  <--- sp
> + * low addr
> + *
> + * The struct stackframe { fp, ra } lives at (fp - sizeof(stackframe)),
> + * i.e. fp[-2]=saved_fp and fp[-1]=saved_ra.
> + *

Maybe link to the authoritative source:

  https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#frame-pointer-convention

> + * For usercopy safety, we allow copies within [prev_fp, fp - 2*sizeof(void*))
> + * for each frame in the chain, where prev_fp is the fp of the previous
> + * (lower) frame.  This covers local variables and arguments but excludes
> + * the saved ra/fp slots at the top of the frame.
> + *
> + * We walk the frame chain starting from __builtin_frame_address(0) (the
> + * current frame), with prev_fp initialized to current_stack_pointer.
> + * Using current_stack_pointer -- rather than the 'stack' argument (which is
> + * the thread's entire stack base) -- ensures that objects in already-returned
> + * frames (address below current sp) are correctly detected as BAD_STACK,
> + * because no live frame in the chain will claim that region.
> + */
> +__no_kmsan_checks
> +static inline int arch_within_stack_frames(const void * const stack,
> +					   const void * const stackend,
> +					   const void *obj, unsigned long len)
> +{
> +#if defined(CONFIG_FRAME_POINTER)
> +	const void *fp = (const void *)__builtin_frame_address(0);
> +	const void *prev_fp = (const void *)current_stack_pointer;
> +
> +	/*
> +	 * Walk the frame chain. Each iteration checks whether [obj, obj+len)
> +	 * falls within the local-variable area of the current frame:
> +	 *
> +	 *   [prev_fp, fp - 2*sizeof(void*))
> +	 *
> +	 * i.e. from the base of this frame (sp of this frame, which equals
> +	 * the fp of the frame below) up to (but not including) the saved
> +	 * fp/ra area at the top of this frame.
> +	 */
> +	while (stack <= fp && fp < stackend) {

Since we access fp - 2 * sizeof(void *) below, this probably should be
something like this:

    while (stack + 2 * sizeof(void *) <= fp && fp < stackend) {

(Possibly with some casts added.)

> +		const void *frame_vars_end = (const char *)fp - 2 * sizeof(void *);
> +
> +		if (obj + len <= frame_vars_end) {
> +			if (obj >= prev_fp)
> +				return GOOD_FRAME;
> +			return BAD_STACK;
> +		}
> +		prev_fp = fp;
> +		fp = *(const void * const *)((const char *)fp - 2 * sizeof(void *));

Maybe: fp = *(const void * const *)frame_vars_end;

> +	}
> +	return BAD_STACK;
> +#else
> +	return NOT_STACK;
> +#endif
> +}
> +#endif /* CONFIG_HAVE_ARCH_WITHIN_STACK_FRAMES */
> +
>  #endif /* !__ASSEMBLER__ */
>  
>  /*

Thanks,
Vivian "dramforever" Wang


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

Re: [PATCH] riscv: mm: Implement arch_within_stack_frames() for HARDENED_USERCOPY

[cp0613]

Hi Vivian,

Thank you very much for your review and suggestions.

On Fri, 10 Apr 2026 15:25:54 +0800, wangruikang@iscas.ac.cn wrote:

> On 4/10/26 09:50, cp0613@linux.alibaba.com wrote:
> > From: Chen Pei <cp0613@linux.alibaba.com>
> >
> > Implement arch_within_stack_frames() to enable precise per-frame stack
> > object validation for CONFIG_HARDENED_USERCOPY on RISC-V.
> >
> > == Background ==
> >
> > [...]
> >
> > Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
> 
> Patch message is probably too long. You can put additional information
> like testing under a --- line to cut those out of the eventual actual
> commit message.
> 
> Make sure that Signed-off-by is above the cut if you do that.

Okay, that's really helpful advice, I've learned a lot.

> > ---
> >  arch/riscv/Kconfig                   |  1 +
> >  arch/riscv/include/asm/thread_info.h | 66 ++++++++++++++++++++++++++++
> >  2 files changed, 67 insertions(+)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 6fe90591a274..4f765ff608d9 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -150,6 +150,7 @@ config RISCV
> >  	select HAVE_ARCH_USERFAULTFD_MINOR if 64BIT && USERFAULTFD
> >  	select HAVE_ARCH_USERFAULTFD_WP if 64BIT && MMU && USERFAULTFD && RISCV_ISA_SVRSW60T59B
> >  	select HAVE_ARCH_VMAP_STACK if MMU && 64BIT
> > +	select HAVE_ARCH_WITHIN_STACK_FRAMES
> >  	select HAVE_ASM_MODVERSIONS
> >  	select HAVE_BUILDTIME_MCOUNT_SORT
> >  	select HAVE_CONTEXT_TRACKING_USER
> > diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> > index 36918c9200c9..616a41eb7416 100644
> > --- a/arch/riscv/include/asm/thread_info.h
> > +++ b/arch/riscv/include/asm/thread_info.h
> > @@ -101,6 +101,72 @@ struct thread_info {
> >  void arch_release_task_struct(struct task_struct *tsk);
> >  int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
> >  
> > +#ifdef CONFIG_HAVE_ARCH_WITHIN_STACK_FRAMES
> 
> Not needed since you've selected it above.

Okay, it seems that it's indeed not necessary.

> > +/*
> > + * RISC-V stack frame layout (with frame pointer enabled):
> > + *
> > + * high addr
> > + *   +------------------+  <--- fp (s0) points here
> > + *   |   saved ra       |  fp - 8  (return address)
> > + *   |   saved fp       |  fp - 16 (previous frame pointer)

> fp - 2*sizeof(void*) probably, to account for RV32?

Yes, You are right. The implementation already uses 2 * sizeof(void *) to
be RV32-compatible. However, the comments still hardcode. I'll update the
comments to use fp - 2*sizeof(void*) to be architecture-neutral.

> > + *   +------------------+
> > + *   |   local vars     |
> > + *   |   arguments      |
> > + *   +------------------+  <--- sp
> > + * low addr
> > + *
> > + * The struct stackframe { fp, ra } lives at (fp - sizeof(stackframe)),
> > + * i.e. fp[-2]=saved_fp and fp[-1]=saved_ra.
> > + *
> 
> Maybe link to the authoritative source:
> 
  > https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#frame-pointer-convention

Okay, I will add this link.

> > + * For usercopy safety, we allow copies within [prev_fp, fp - 2*sizeof(void*))
> > + * for each frame in the chain, where prev_fp is the fp of the previous
> > + * (lower) frame.  This covers local variables and arguments but excludes
> > + * the saved ra/fp slots at the top of the frame.
> > + *
> > + * We walk the frame chain starting from __builtin_frame_address(0) (the
> > + * current frame), with prev_fp initialized to current_stack_pointer.
> > + * Using current_stack_pointer -- rather than the 'stack' argument (which is
> > + * the thread's entire stack base) -- ensures that objects in already-returned
> > + * frames (address below current sp) are correctly detected as BAD_STACK,
> > + * because no live frame in the chain will claim that region.
> > + */
> > +__no_kmsan_checks
> > +static inline int arch_within_stack_frames(const void * const stack,
> > +					   const void * const stackend,
> > +					   const void *obj, unsigned long len)
> > +{
> > +#if defined(CONFIG_FRAME_POINTER)
> > +	const void *fp = (const void *)__builtin_frame_address(0);
> > +	const void *prev_fp = (const void *)current_stack_pointer;
> > +
> > +	/*
> > +	 * Walk the frame chain. Each iteration checks whether [obj, obj+len)
> > +	 * falls within the local-variable area of the current frame:
> > +	 *
> > +	 *   [prev_fp, fp - 2*sizeof(void*))
> > +	 *
> > +	 * i.e. from the base of this frame (sp of this frame, which equals
> > +	 * the fp of the frame below) up to (but not including) the saved
> > +	 * fp/ra area at the top of this frame.
> > +	 */
> > +	while (stack <= fp && fp < stackend) {
> 
> Since we access fp - 2 * sizeof(void *) below, this probably should be
> something like this:
> 
    > while (stack + 2 * sizeof(void *) <= fp && fp < stackend) {
> 
> (Possibly with some casts added.)

Thank you for pointing it out; I hadn't considered the potential for
boundary violations here.

> > +		const void *frame_vars_end = (const char *)fp - 2 * sizeof(void *);
> > +
> > +		if (obj + len <= frame_vars_end) {
> > +			if (obj >= prev_fp)
> > +				return GOOD_FRAME;
> > +			return BAD_STACK;
> > +		}
> > +		prev_fp = fp;
> > +		fp = *(const void * const *)((const char *)fp - 2 * sizeof(void *));
> 
> Maybe: fp = *(const void * const *)frame_vars_end;

Yes, direct reuse.

> > +	}
> > +	return BAD_STACK;
> > +#else
> > +	return NOT_STACK;
> > +#endif
> > +}
> > +#endif /* CONFIG_HAVE_ARCH_WITHIN_STACK_FRAMES */
> > +
> >  #endif /* !__ASSEMBLER__ */
> >  
> >  /*

> Thanks,
> Vivian "dramforever" Wang

Thanks,
Pei

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