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

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

[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 <mark.rutland@arm.com>
Cc: <stable@vger.kernel.org>
Fixes: 18011eac28c7 ("arm64: tls: Avoid unconditional zeroing of tpidrro_el0 for native tasks")
Signed-off-by: Will Deacon <will@kernel.org>
---

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

 arch/arm64/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 3e7c8c8195c3..2bbcbb11d844 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -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);
-- 
2.47.0.277.g8800431eea-goog

Re: [PATCH] arm64: tls: Fix context-switching of tpidrro_el0 when kpti is enabled

[Mark Rutland]

On Thu, Nov 14, 2024 at 09:53:32AM +0000, Will Deacon wrote:
> 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 <mark.rutland@arm.com>
> Cc: <stable@vger.kernel.org>
> Fixes: 18011eac28c7 ("arm64: tls: Avoid unconditional zeroing of tpidrro_el0 for native tasks")
> Signed-off-by: Will Deacon <will@kernel.org>

Acked-by: Mark Rutland <mark.rutland@arm.com>

Mark.

> ---
> 
> You fix one side-channel and introduce another... :(
> 
>  arch/arm64/kernel/process.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 3e7c8c8195c3..2bbcbb11d844 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -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);
> -- 
> 2.47.0.277.g8800431eea-goog
> 

Re: [PATCH] arm64: tls: Fix context-switching of tpidrro_el0 when kpti is enabled

[Marc Zyngier]

On Thu, 14 Nov 2024 09:53:32 +0000,
Will Deacon <will@kernel.org> wrote:
> 
> 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 <mark.rutland@arm.com>
> Cc: <stable@vger.kernel.org>
> Fixes: 18011eac28c7 ("arm64: tls: Avoid unconditional zeroing of tpidrro_el0 for native tasks")
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
> 
> You fix one side-channel and introduce another... :(
>
>  arch/arm64/kernel/process.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 3e7c8c8195c3..2bbcbb11d844 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -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 <maz@kernel.org>

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH] arm64: tls: Fix context-switching of tpidrro_el0 when kpti is enabled

[Catalin Marinas]

On Thu, 14 Nov 2024 09:53:32 +0000, Will Deacon wrote:
> 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

-- 
Catalin