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

[Ben Dooks <ben.dooks@codethink.co.uk>]

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


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

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