From: Peter Collingbourne <hidden> Date: 2021-09-29 19:47:45
On some microarchitectures, clearing PSTATE.TCO is expensive. Clearing
TCO is only necessary if in-kernel MTE is enabled, or if MTE is
enabled in the userspace process in synchronous (or, soon, asymmetric)
mode, because we do not report uaccess faults to userspace in none
or asynchronous modes. Therefore, adjust the kernel entry code to
clear TCO only if necessary.
Signed-off-by: Peter Collingbourne <redacted>
Link: https://linux-review.googlesource.com/id/I52d82a580bd0500d420be501af2c35fa8c90729e
---
arch/arm64/kernel/entry.S | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
I think we can get here from an interrupt as well. Can we guarantee that
the sctlr_user is valid? We are not always in a user process context.
Maybe only do the above checks if \el == 0, otherwise just bracket it
with kasan_hw_tags_enable.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
I think we can get here from an interrupt as well. Can we guarantee that
the sctlr_user is valid? We are not always in a user process context.
Looking through the code in entry.S carefully it doesn't appear that
the tsk pointer is ever used when taking an exception from EL1. The
last user appears to have been removed in commit 3d2403fd10a1 ("arm64:
uaccess: remove set_fs()"). I just did a quick boot test on a couple
of platforms and things seem to work without the "get_current_task
tsk" line.
So I can't be confident that tsk points to a valid task, but at least
it looks like it was the case prior to that commit.
Maybe only do the above checks if \el == 0, otherwise just bracket it
with kasan_hw_tags_enable.
Is it possible for us to do a uaccess inside the EL1 -> EL1 exception
handler? That was the one thing I was unsure about and it's why I
opted to do the same check regardless of EL.
If we don't think these uaccesses can happen, or we don't think it
matters that they will either fail or not depending on whether KASAN
is enabled, then I think we can make it dependent on only
kasan_hw_tags_enable.
Peter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
I think we can get here from an interrupt as well. Can we guarantee that
the sctlr_user is valid? We are not always in a user process context.
Looking through the code in entry.S carefully it doesn't appear that
the tsk pointer is ever used when taking an exception from EL1. The
last user appears to have been removed in commit 3d2403fd10a1 ("arm64:
uaccess: remove set_fs()"). I just did a quick boot test on a couple
of platforms and things seem to work without the "get_current_task
tsk" line.
So I can't be confident that tsk points to a valid task, but at least
it looks like it was the case prior to that commit.
quoted
Maybe only do the above checks if \el == 0, otherwise just bracket it
with kasan_hw_tags_enable.
Is it possible for us to do a uaccess inside the EL1 -> EL1 exception
handler? That was the one thing I was unsure about and it's why I
opted to do the same check regardless of EL.
Today we can do so if we take a PMU IRQ from EL1 and try to do an EL0
stack unwind or stack dump, and similar holds for some eBPF stuff.
We also need to ensure that PSTATE.TCO is configured consistently so
that context-switch works, otherwise where a CPU switches between tasks
A and B, where A is preempted by an EL1 IRQ, and B is explicitly
switching via a direct call to schedule(), the state of TCO will not be
as expected (unless we track this per thread, and handle it in the
context switch).
I'd strongly prefer that the state of PSTATE.TCO upon taking an
exception to EL1 is consistent (by the end of the early entry code),
regardless of where that was taken from.
Is it easier to manage this from within entry-common.c?
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
I think we can get here from an interrupt as well. Can we guarantee that
the sctlr_user is valid? We are not always in a user process context.
Looking through the code in entry.S carefully it doesn't appear that
the tsk pointer is ever used when taking an exception from EL1. The
last user appears to have been removed in commit 3d2403fd10a1 ("arm64:
uaccess: remove set_fs()"). I just did a quick boot test on a couple
of platforms and things seem to work without the "get_current_task
tsk" line.
So I can't be confident that tsk points to a valid task, but at least
it looks like it was the case prior to that commit.
The EL1 entries unconditionally call enter_from_kernel_mode() which
(at least in some configurations) unconditionally accesses current, so
I'm reasonably confident that we have a valid current pointer here.
quoted
quoted
Maybe only do the above checks if \el == 0, otherwise just bracket it
with kasan_hw_tags_enable.
Is it possible for us to do a uaccess inside the EL1 -> EL1 exception
handler? That was the one thing I was unsure about and it's why I
opted to do the same check regardless of EL.
Today we can do so if we take a PMU IRQ from EL1 and try to do an EL0
stack unwind or stack dump, and similar holds for some eBPF stuff.
We also need to ensure that PSTATE.TCO is configured consistently so
that context-switch works, otherwise where a CPU switches between tasks
A and B, where A is preempted by an EL1 IRQ, and B is explicitly
switching via a direct call to schedule(), the state of TCO will not be
as expected (unless we track this per thread, and handle it in the
context switch).
Isn't this already context switched via storing the per-task SPSR?
Otherwise e.g. the TCO controls in __uaccess_disable_tco() and
__uaccess_enable_tco() would have the same problem.
I'd strongly prefer that the state of PSTATE.TCO upon taking an
exception to EL1 is consistent (by the end of the early entry code),
regardless of where that was taken from.
Is it easier to manage this from within entry-common.c?
It doesn't look like we have any common location to add code that runs
on every kernel entry in entry-common.c. I tried adding one (patch
below), but this ended up leading to a performance regression (versus
baseline) on some of the cores on the hardware that I have access to.
Peter
I think we can get here from an interrupt as well. Can we guarantee that
the sctlr_user is valid? We are not always in a user process context.
Looking through the code in entry.S carefully it doesn't appear that
the tsk pointer is ever used when taking an exception from EL1. The
last user appears to have been removed in commit 3d2403fd10a1 ("arm64:
uaccess: remove set_fs()"). I just did a quick boot test on a couple
of platforms and things seem to work without the "get_current_task
tsk" line.
So I can't be confident that tsk points to a valid task, but at least
it looks like it was the case prior to that commit.
The EL1 entries unconditionally call enter_from_kernel_mode() which
(at least in some configurations) unconditionally accesses current, so
I'm reasonably confident that we have a valid current pointer here.
That's the value in SP_EL0, which is valid so long as we never take an
exception from the entry assembly itself. We restore that from __entry_task in
the EL0 entry paths, and context-switch it when switching threads.
I would like to get rid of `tsk` in the entry assembly, but that requires
moving more of that assembly to C.
quoted
quoted
quoted
Maybe only do the above checks if \el == 0, otherwise just bracket it
with kasan_hw_tags_enable.
Is it possible for us to do a uaccess inside the EL1 -> EL1 exception
handler? That was the one thing I was unsure about and it's why I
opted to do the same check regardless of EL.
Today we can do so if we take a PMU IRQ from EL1 and try to do an EL0
stack unwind or stack dump, and similar holds for some eBPF stuff.
We also need to ensure that PSTATE.TCO is configured consistently so
that context-switch works, otherwise where a CPU switches between tasks
A and B, where A is preempted by an EL1 IRQ, and B is explicitly
switching via a direct call to schedule(), the state of TCO will not be
as expected (unless we track this per thread, and handle it in the
context switch).
Isn't this already context switched via storing the per-task SPSR?
Otherwise e.g. the TCO controls in __uaccess_disable_tco() and
__uaccess_enable_tco() would have the same problem.
That's the case for pre-emptive scheduling off the back of an interrupt, but
tasks can call into schedule without an intervening exception, e.g. blocking
on something like a mutex. Those blocking primitives aren't permitted to be
used within a __uaccess_{disable,enable)_tco() critical section.
quoted
I'd strongly prefer that the state of PSTATE.TCO upon taking an
exception to EL1 is consistent (by the end of the early entry code),
regardless of where that was taken from.
Is it easier to manage this from within entry-common.c?
It doesn't look like we have any common location to add code that runs
on every kernel entry in entry-common.c.
I was thinking we could rework the existing mte_check_tfsr_*() callsites into
mte_enter_from_{user,kernel}() hooks, which'd allow us to avoid redundant checks
for MTE support.
I tried adding one (patch
below), but this ended up leading to a performance regression (versus
baseline) on some of the cores on the hardware that I have access to.
How big a regression? On a real workload or a microbenchmark?
Is that true for a simple move of the current logic to C, i.e. for an
unconditional SET_PSTATE_TCO(0) gated by cpus_have_final_cap(ARM64_MTE) ?
Thanks,
Mark.
I think we can get here from an interrupt as well. Can we guarantee that
the sctlr_user is valid? We are not always in a user process context.
Looking through the code in entry.S carefully it doesn't appear that
the tsk pointer is ever used when taking an exception from EL1. The
last user appears to have been removed in commit 3d2403fd10a1 ("arm64:
uaccess: remove set_fs()"). I just did a quick boot test on a couple
of platforms and things seem to work without the "get_current_task
tsk" line.
So I can't be confident that tsk points to a valid task, but at least
it looks like it was the case prior to that commit.
The EL1 entries unconditionally call enter_from_kernel_mode() which
(at least in some configurations) unconditionally accesses current, so
I'm reasonably confident that we have a valid current pointer here.
That's the value in SP_EL0, which is valid so long as we never take an
exception from the entry assembly itself. We restore that from __entry_task in
the EL0 entry paths, and context-switch it when switching threads.
I would like to get rid of `tsk` in the entry assembly, but that requires
moving more of that assembly to C.
quoted
quoted
quoted
quoted
Maybe only do the above checks if \el == 0, otherwise just bracket it
with kasan_hw_tags_enable.
Is it possible for us to do a uaccess inside the EL1 -> EL1 exception
handler? That was the one thing I was unsure about and it's why I
opted to do the same check regardless of EL.
Today we can do so if we take a PMU IRQ from EL1 and try to do an EL0
stack unwind or stack dump, and similar holds for some eBPF stuff.
We also need to ensure that PSTATE.TCO is configured consistently so
that context-switch works, otherwise where a CPU switches between tasks
A and B, where A is preempted by an EL1 IRQ, and B is explicitly
switching via a direct call to schedule(), the state of TCO will not be
as expected (unless we track this per thread, and handle it in the
context switch).
Isn't this already context switched via storing the per-task SPSR?
Otherwise e.g. the TCO controls in __uaccess_disable_tco() and
__uaccess_enable_tco() would have the same problem.
That's the case for pre-emptive scheduling off the back of an interrupt, but
tasks can call into schedule without an intervening exception, e.g. blocking
on something like a mutex. Those blocking primitives aren't permitted to be
used within a __uaccess_{disable,enable)_tco() critical section.
I see. I guess you wouldn't even need an EL1 IRQ to expose the bug
here -- you could schedule() directly from A to B where A is NONE and
B is SYNC. I think we can handle this by clearing TCO in
cpu_switch_to() with the same condition as for exception entry; done
in v2.
quoted
quoted
I'd strongly prefer that the state of PSTATE.TCO upon taking an
exception to EL1 is consistent (by the end of the early entry code),
regardless of where that was taken from.
Is it easier to manage this from within entry-common.c?
It doesn't look like we have any common location to add code that runs
on every kernel entry in entry-common.c.
I was thinking we could rework the existing mte_check_tfsr_*() callsites into
mte_enter_from_{user,kernel}() hooks, which'd allow us to avoid redundant checks
for MTE support.
We currently only check TFSR on entry from EL1 (which makes sense,
since we would not expect an entry from EL0 to have triggered an EL1
tag check fault earlier). So it seems like we'd want slightly
different code for the two ELs anyway.
quoted
I tried adding one (patch
below), but this ended up leading to a performance regression (versus
baseline) on some of the cores on the hardware that I have access to.
How big a regression? On a real workload or a microbenchmark?