Re: [PATCH] riscv: Fix SR_SUM leakage across context switches

[Andy Chiu <tchiu@tenstorrent.com>]

Hi Yunhui,

On Thu, Oct 22, 2026 at 04:32:12PM +0800, Yunhui Cui wrote:
> __switch_to() saves the sstatus value of prev and restores the SR_SUM bit
> of next with csrs. However, csrs only sets bits. If prev has SR_SUM set
> and next has SR_SUM clear in its saved state, the old SR_SUM bit remains
> set in sstatus.
> 
> This lets next run with user memory access enabled even though its saved
> SR_SUM state is clear.
> 
> Use csrrc to save the sstatus value of prev and clear SR_SUM from the
> live CSR before restoring the saved SR_SUM state of next. This keeps
> prev's SR_SUM state intact, while preventing it from leaking to next.

Do you have a way to trigger this bug on existing code or is it the cae
where the bug could happen but we never experienced it in real life?

If I understand correctly, this can only happen when the user access
routing volutarily makes a schedule() call mid-flight, or a code fogets
to turn off SR_SUM right after an user access and has a live SR_SUM when
calling schedule some time after. Both case should be a bug as SR_SUM
shouldn't live in the kernel code that is not doing user access.

A trap-introduced context switch would interrupt a code that have SR_SUM
set. However, since the bit is cleared at every exception entry, the
context switch code would not see a live SR_SUM. Seeing the bit set in
this case means something is not turning SR_SUM off properly.

I aggree that this is the better way to handle SR_SUM in a context switch
but attaching a call trace or stating that "We have not caught this in
real world due to ..." in the commit message would be appreciated.

Cheers,
Andy

> 
> Fixes: 788aa64c01f1 ("riscv: save the SR_SUM status over switches")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>  arch/riscv/kernel/entry.S | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index 61b7a77166971..628b85e3915a7 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -443,15 +443,15 @@ SYM_FUNC_START(__switch_to)
>  	REG_S s10, TASK_THREAD_S10_RA(a3)
>  	REG_S s11, TASK_THREAD_S11_RA(a3)
>  
> -	/* save the user space access flag */
> -	csrr  s0, CSR_STATUS
> +	/* Save and clear the user space access flag. */
> +	li    s1, SR_SUM
> +	csrrc s0, CSR_STATUS, s1
>  	REG_S s0, TASK_THREAD_SUM_RA(a3)
>  
>  	/* Save the kernel shadow call stack pointer */
>  	scs_save_current
>  	/* Restore context from next->thread */
>  	REG_L s0,  TASK_THREAD_SUM_RA(a4)
> -	li    s1,  SR_SUM
>  	and   s0,  s0, s1
>  	csrs  CSR_STATUS, s0
>  	REG_L ra,  TASK_THREAD_RA_RA(a4)
> -- 
> 2.39.5
>