Skip to contentSkip to conversation

arm64: tls: Fix context-switching of tpidrro_el0 when kpti is enabled

PATCH Applied Will Deacon 4 participants 4 messages Nov 14, 2024

Subsystems (2)
Stable branch arm64
lore
Share & tools
lore

Will Deacon

author maintainer
#0 lore raw

PATCH 1/1 arm64: tls: Fix context-switching of tpidrro_el0 when kpti is enabled

Commit 18011eac28c7 ("arm64: tls: Avoid unconditional zeroing of tpidrro_el0 for native tasks") tried to optimise the context switching of tpidrro_el0 by eliding the clearing of the register when switching to a native task with kpti enabled, on the erroneous assumption that the kpti trampoline entry code would already have taken care of the write.

Although the kpti trampoline does zero the register on entry from a native task, the check in tls_thread_switch() is on the *next* task and so we can end up leaving a stale, non-zero value in the register if the previous task was 32-bit.

Drop the broken optimisation and zero tpidrro_el0 unconditionally when switching to a native 64-bit task.

Cc Mark Rutland <[email protected]>
Cc <[email protected]>
Fixes 18011eac28c7 ("arm64: tls: Avoid unconditional zeroing of tpidrro_el0 for native tasks")
Signed-off-by Will Deacon <[email protected]>

---

You fix one side-channel and introduce another... :(

 arch/arm64/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/process.c +1 -1
index 3e7c8c8195c3..2bbcbb11d844 100644
@@ -442,7 +442,7 @@ static void tls_thread_switch(struct task_struct *next)
 
 	if (is_compat_thread(task_thread_info(next)))
 		write_sysreg(next->thread.uw.tp_value, tpidrro_el0);
-	else if (!arm64_kernel_unmapped_at_el0())
+	else
 		write_sysreg(0, tpidrro_el0);
 
 	write_sysreg(*task_user_tls(next), tpidr_el0);
Signature · 1 line
2.47.0.277.g8800431eea-goog
reviewer Acked-by on v1 1/1
#1 lore raw
Quoted 12 lines · Will Deacon

Commit 18011eac28c7 ("arm64: tls: Avoid unconditional zeroing of tpidrro_el0 for native tasks") tried to optimise the context switching of tpidrro_el0 by eliding the clearing of the register when switching to a native task with kpti enabled, on the erroneous assumption that the kpti trampoline entry code would already have taken care of the write.

Although the kpti trampoline does zero the register on entry from a native task, the check in tls_thread_switch() is on the *next* task and so we can end up leaving a stale, non-zero value in the register if the previous task was 32-bit.

Quoted 7 lines

Drop the broken optimisation and zero tpidrro_el0 unconditionally when switching to a native 64-bit task.

Cc Mark Rutland <[email protected]>
Cc <[email protected]>
Fixes 18011eac28c7 ("arm64: tls: Avoid unconditional zeroing of tpidrro_el0 for native tasks")
Signed-off-by Will Deacon <[email protected]>
Acked-by Mark Rutland <[email protected]>

Mark.

Quoted 23 lines

---

You fix one side-channel and introduce another... :(

 arch/arm64/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/process.c +1 -1
index 3e7c8c8195c3..2bbcbb11d844 100644
@@ -442,7 +442,7 @@ static void tls_thread_switch(struct task_struct *next)
 
 	if (is_compat_thread(task_thread_info(next)))
 		write_sysreg(next->thread.uw.tp_value, tpidrro_el0);
-	else if (!arm64_kernel_unmapped_at_el0())
+	else
 		write_sysreg(0, tpidrro_el0);
 
 	write_sysreg(*task_user_tls(next), tpidr_el0);
Signature · 1 line
2.47.0.277.g8800431eea-goog
Acked-by on v1 1/1
#2 lore raw
In reply to Will Deacon
Quoted 32 lines · Will Deacon

Commit 18011eac28c7 ("arm64: tls: Avoid unconditional zeroing of tpidrro_el0 for native tasks") tried to optimise the context switching of tpidrro_el0 by eliding the clearing of the register when switching to a native task with kpti enabled, on the erroneous assumption that the kpti trampoline entry code would already have taken care of the write.

Although the kpti trampoline does zero the register on entry from a native task, the check in tls_thread_switch() is on the *next* task and so we can end up leaving a stale, non-zero value in the register if the previous task was 32-bit.

Drop the broken optimisation and zero tpidrro_el0 unconditionally when switching to a native 64-bit task.

Cc Mark Rutland <[email protected]>
Cc <[email protected]>
Fixes 18011eac28c7 ("arm64: tls: Avoid unconditional zeroing of tpidrro_el0 for native tasks")
Signed-off-by Will Deacon <[email protected]>

---

You fix one side-channel and introduce another... :(

 arch/arm64/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kernel/process.c +0 -0
index 3e7c8c8195c3..2bbcbb11d844 100644

arch/arm64/kernel/process.c static void tls_thread_switch(struct task_struct *next) · line 448

Quoted 8 lines
arch/arm64/kernel/process.c +1 -1
@@ -442,7 +442,7 @@ static void tls_thread_switch(struct task_struct *next)
 
 	if (is_compat_thread(task_thread_info(next)))
 		write_sysreg(next->thread.uw.tp_value, tpidrro_el0);
-	else if (!arm64_kernel_unmapped_at_el0())
+	else
 		write_sysreg(0, tpidrro_el0);
 
 	write_sysreg(*task_user_tls(next), tpidr_el0);
Acked-by Marc Zyngier <[email protected]>
	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
maintainer Applied
#3 lore raw
In reply to Will Deacon
Quoted 8 lines · Will Deacon

Commit 18011eac28c7 ("arm64: tls: Avoid unconditional zeroing of tpidrro_el0 for native tasks") tried to optimise the context switching of tpidrro_el0 by eliding the clearing of the register when switching to a native task with kpti enabled, on the erroneous assumption that the kpti trampoline entry code would already have taken care of the write.

[...]

Applied to arm64 (for-next/misc), thanks!

[1/1] arm64: tls: Fix context-switching of tpidrro_el0 when kpti is enabled

      https://git.kernel.org/arm64/c/67ab51cbdfee
Signature · 1 line
Catalin