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

[Ben Dooks]

On 22/10/2026 23:19, Andy Chiu wrote:
> 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?

The original case for dealing with this was due to the user-access
macros occasionally embedding calls into the area where the SR_SUM
was set, allowing pre-emption there. We did try and fix this and as
far as I knew then it was mostly fixed by re-ordering the uaccess
macros.

> 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.

I'm not sure how much code does a SR_SUM enable a few different accesses
and then SR_SUM disable, but IIRC there is a possibility to do this.

> 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@xxxxxxxxxxxxxxx
> > Signed-off-by: Yunhui Cui <cuiyunhui@xxxxxxxxxxxxx>
> > ---
> >   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


--
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html