Skip to content

Commit 19c95f2

Browse files
Julien Thierrywilldeacon
authored andcommitted
arm64: entry.S: Do not preempt from IRQ before all cpufeatures are enabled
Preempting from IRQ-return means that the task has its PSTATE saved on the stack, which will get restored when the task is resumed and does the actual IRQ return. However, enabling some CPU features requires modifying the PSTATE. This means that, if a task was scheduled out during an IRQ-return before all CPU features are enabled, the task might restore a PSTATE that does not include the feature enablement changes once scheduled back in. * Task 1: PAN == 0 ---| |--------------- | |<- return from IRQ, PSTATE.PAN = 0 | <- IRQ | +--------+ <- preempt() +-- ^ | reschedule Task 1, PSTATE.PAN == 1 * Init: --------------------+------------------------ ^ | enable_cpu_features set PSTATE.PAN on all CPUs Worse than this, since PSTATE is untouched when task switching is done, a task missing the new bits in PSTATE might affect another task, if both do direct calls to schedule() (outside of IRQ/exception contexts). Fix this by preventing preemption on IRQ-return until features are enabled on all CPUs. This way the only PSTATE values that are saved on the stack are from synchronous exceptions. These are expected to be fatal this early, the exception is BRK for WARN_ON(), but as this uses do_debug_exception() which keeps IRQs masked, it shouldn't call schedule(). Signed-off-by: Julien Thierry <[email protected]> [james: Replaced a really cool hack, with an even simpler static key in C. expanded commit message with Julien's cover-letter ascii art] Signed-off-by: James Morse <[email protected]> Signed-off-by: Will Deacon <[email protected]>
1 parent 8c551f9 commit 19c95f2

File tree

3 files changed

+20
-1
lines changed

3 files changed

+20
-1
lines changed

arch/arm64/kernel/entry.S

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,7 @@ alternative_if ARM64_HAS_IRQ_PRIO_MASKING
680680
orr x24, x24, x0
681681
alternative_else_nop_endif
682682
cbnz x24, 1f // preempt count != 0 || NMI return path
683-
bl preempt_schedule_irq // irq en/disable is done inside
683+
bl arm64_preempt_schedule_irq // irq en/disable is done inside
684684
1:
685685
#endif
686686

arch/arm64/kernel/process.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <linux/sched/task.h>
1818
#include <linux/sched/task_stack.h>
1919
#include <linux/kernel.h>
20+
#include <linux/lockdep.h>
2021
#include <linux/mm.h>
2122
#include <linux/stddef.h>
2223
#include <linux/sysctl.h>
@@ -44,6 +45,7 @@
4445
#include <asm/alternative.h>
4546
#include <asm/arch_gicv3.h>
4647
#include <asm/compat.h>
48+
#include <asm/cpufeature.h>
4749
#include <asm/cacheflush.h>
4850
#include <asm/exec.h>
4951
#include <asm/fpsimd.h>
@@ -631,3 +633,19 @@ static int __init tagged_addr_init(void)
631633

632634
core_initcall(tagged_addr_init);
633635
#endif /* CONFIG_ARM64_TAGGED_ADDR_ABI */
636+
637+
asmlinkage void __sched arm64_preempt_schedule_irq(void)
638+
{
639+
lockdep_assert_irqs_disabled();
640+
641+
/*
642+
* Preempting a task from an IRQ means we leave copies of PSTATE
643+
* on the stack. cpufeature's enable calls may modify PSTATE, but
644+
* resuming one of these preempted tasks would undo those changes.
645+
*
646+
* Only allow a task to be preempted once cpufeatures have been
647+
* enabled.
648+
*/
649+
if (static_branch_likely(&arm64_const_caps_ready))
650+
preempt_schedule_irq();
651+
}

include/linux/sched.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ extern long schedule_timeout_uninterruptible(long timeout);
223223
extern long schedule_timeout_idle(long timeout);
224224
asmlinkage void schedule(void);
225225
extern void schedule_preempt_disabled(void);
226+
asmlinkage void preempt_schedule_irq(void);
226227

227228
extern int __must_check io_schedule_prepare(void);
228229
extern void io_schedule_finish(int token);

0 commit comments

Comments
 (0)