Skip to content

Commit 2abf7ce

Browse files
dcpleungkartben
authored andcommitted
xtensa: userspace: prevent potential privilege escalation
When context switching and dealing with non-nested interrupts, the context to be restored are saved in the thread stack. When userspace is enabled, this means saving context into the user stacks for user threads. This allows PS values to be manipulated externally by setting PS.RING in the saved PS value to 0, resulting in granting kernel access privilege when the thread is restored. To prevent this, we store the PS value into the thread struct instead, where user threads cannot manipulate that. Note that nested interrupts and syscalls are not using the user stack but the interrupt stack and thread privileged stack respectively, where they are not accessible under user mode. Signed-off-by: Daniel Leung <daniel.leung@intel.com>
1 parent d5c2e4b commit 2abf7ce

File tree

6 files changed

+73
-1
lines changed

6 files changed

+73
-1
lines changed

arch/xtensa/core/offsets/offsets.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ GEN_OFFSET_SYM(_xtensa_irq_bsa_t, hifi);
6767

6868
#ifdef CONFIG_USERSPACE
6969
GEN_OFFSET_SYM(_thread_arch_t, psp);
70+
GEN_OFFSET_SYM(_thread_arch_t, return_ps);
71+
GEN_OFFSET_SYM(_thread_t, switch_handle);
7072
#ifdef CONFIG_XTENSA_MMU
7173
GEN_OFFSET_SYM(_thread_arch_t, ptables);
7274
#endif

arch/xtensa/core/thread.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,19 @@ static void *init_stack(struct k_thread *thread, int *stack_top,
6666

6767
(void)memset(frame, 0, bsasz);
6868

69-
frame->bsa.ps = PS_WOE | PS_UM | PS_CALLINC(1);
7069
#ifdef CONFIG_USERSPACE
70+
/* _restore_context uses this instead of frame->bsa.ps to
71+
* restore PS value.
72+
*/
73+
thread->arch.return_ps = PS_WOE | PS_UM | PS_CALLINC(1);
74+
7175
if ((thread->base.user_options & K_USER) == K_USER) {
7276
frame->bsa.pc = (uintptr_t)arch_user_mode_enter;
7377
} else {
7478
frame->bsa.pc = (uintptr_t)z_thread_entry;
7579
}
7680
#else
81+
frame->bsa.ps = PS_WOE | PS_UM | PS_CALLINC(1);
7782
frame->bsa.pc = (uintptr_t)z_thread_entry;
7883
#endif
7984

arch/xtensa/core/xtensa_asm2_util.S

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,34 @@ _restore_context:
153153

154154
l32i a0, a1, ___xtensa_irq_bsa_t_pc_OFFSET
155155
wsr a0, ZSR_EPC
156+
157+
#ifdef CONFIG_USERSPACE
158+
/* When restoring context via xtensa_switch and
159+
* returning from non-nested interrupts, we use
160+
* the stashed PS value in the thread struct
161+
* instead of the one in the thread stack.
162+
* Both scenarios will have nested value of 0.
163+
*/
164+
rsr.ZSR_CPU a2
165+
l32i a0, a2, ___cpu_t_nested_OFFSET
166+
bnez a0, _restore_ps_from_stack
167+
168+
l32i a0, a2, ___cpu_t_current_OFFSET
169+
l32i a0, a0, _thread_offset_to_return_ps
170+
wsr a0, ZSR_EPS
171+
172+
j _restore_ps_after
173+
174+
_restore_ps_from_stack:
175+
#endif
176+
156177
l32i a0, a1, ___xtensa_irq_bsa_t_ps_OFFSET
157178
wsr a0, ZSR_EPS
158179

180+
#ifdef CONFIG_USERSPACE
181+
_restore_ps_after:
182+
#endif
183+
159184
#if XCHAL_HAVE_FP && defined(CONFIG_CPU_HAS_FPU) && defined(CONFIG_FPU_SHARING)
160185
FPU_REG_RESTORE
161186
#endif
@@ -262,6 +287,18 @@ xtensa_switch:
262287
/* Stash our PS register contents and a "restore" PC. */
263288
rsr a0, PS
264289
s32i a0, a1, ___xtensa_irq_bsa_t_ps_OFFSET
290+
291+
#ifdef CONFIG_USERSPACE
292+
/* Backtrack to the head of thread struct and
293+
* then store the PS value to be restored in
294+
* the architecture specific section.
295+
* This will be used to restore PS instead of
296+
* the one stashed inside stack.
297+
*/
298+
addi a3, a3, -___thread_t_switch_handle_OFFSET
299+
s32i a0, a3, _thread_offset_to_return_ps
300+
#endif
301+
265302
movi a0, _switch_restore_pc
266303
s32i a0, a1, ___xtensa_irq_bsa_t_pc_OFFSET
267304

arch/xtensa/include/offsets_short_arch.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
#define _thread_offset_to_psp \
1313
(___thread_t_arch_OFFSET + ___thread_arch_t_psp_OFFSET)
1414

15+
#define _thread_offset_to_return_ps \
16+
(___thread_t_arch_OFFSET + ___thread_arch_t_return_ps_OFFSET)
17+
1518
#define _thread_offset_to_ptables \
1619
(___thread_t_arch_OFFSET + ___thread_arch_t_ptables_OFFSET)
1720
#endif /* CONFIG_USERSPACE */

arch/xtensa/include/xtensa_asm2_s.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "xtensa_asm2_context.h"
1212

1313
#include <zephyr/offsets.h>
14+
#include <offsets_short.h>
1415

1516
/* Assembler header! This file contains macros designed to be included
1617
* only by the assembler.
@@ -686,6 +687,25 @@ _Level\LVL\()Vector:
686687
s32i a0, a1, ___xtensa_irq_bsa_t_ps_OFFSET
687688
.endif
688689

690+
#ifdef CONFIG_USERSPACE
691+
/* When restoring context via xtensa_switch and
692+
* returning from non-nested interrupts, we will be
693+
* using the stashed PS value in the thread struct
694+
* instead of the one in the thread stack. Both of
695+
* these scenarios will have nested value of 0.
696+
* So when nested value is zero, we store the PS
697+
* value into thread struct.
698+
*/
699+
rsr.ZSR_CPU a3
700+
l32i a2, a3, ___cpu_t_nested_OFFSET
701+
bnez a2, _excint_skip_ps_save_to_thread_\LVL
702+
703+
l32i a2, a3, ___cpu_t_current_OFFSET
704+
s32i a0, a2, _thread_offset_to_return_ps
705+
706+
_excint_skip_ps_save_to_thread_\LVL:
707+
#endif
708+
689709
rsr.epc\LVL a0
690710
s32i a0, a1, ___xtensa_irq_bsa_t_pc_OFFSET
691711

include/zephyr/arch/xtensa/thread.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ struct _thread_arch {
4242
* Un-set for surpervisor threads.
4343
*/
4444
uint8_t *psp;
45+
46+
/* Stashed PS value used to restore PS when restoring from
47+
* context switching or returning from non-nested interrupts.
48+
*/
49+
uint32_t return_ps;
4550
#endif
4651
};
4752

0 commit comments

Comments
 (0)