+Kees and Jorge and Jann On Tue, Oct 29, 2024 at 7:45 AM Kevin Brodsky <kevin.brod...@arm.com> wrote: > > This series is a follow-up to Joey's Permission Overlay Extension (POE) > series [1] that recently landed on mainline. The goal is to improve the > way we handle the register that governs which pkeys/POIndex are > accessible (POR_EL0) during signal delivery. As things stand, we may > unexpectedly fail to write the signal frame on the stack because POR_EL0 > is not reset before the uaccess operations. See patch 1 for more details > and the main changes this series brings. > > A similar series landed recently for x86/MPK [2]; the present series > aims at aligning arm64 with x86. Worth noting: once the signal frame is > written, POR_EL0 is still set to POR_EL0_INIT, granting access to pkey 0 > only. This means that a program that sets up an alternate signal stack > with a non-zero pkey will need some assembly trampoline to set POR_EL0 > before invoking the real signal handler, as discussed here [3]. This is > not ideal, but it makes experimentation with pkeys in signal handlers > possible while waiting for a potential interface to control the pkey > state when delivering a signal. See Pierre's reply [4] for more > information about use-cases and a potential interface. > Apologize in advance that I'm unfamiliar with ARM's POR, up to review this patch series, so I might ask silly questions or based on wrong understanding.
It seems that the patch has the same logic as Aruna Ramakrishna proposed for X86, is this correct ? In the latest version of x86 change [1], I have a comment if we want to consider adding a new flag SS_PKEYALTSTACK (see SS_AUTODISARM as an example) in sigaltstack, and restrict this mechanism (overwriting PKRU/POR_EL0 and sigframe) to sigaltstack() with SS_PKEYALTSTACK. There is a subtle difference if we do that, i.e. the existing signaling handling user might not care or do not use PKEY/POE, and overwriting PKRU/POR_EL0 and sigframe every time will add extra CPU time on the signaling delivery, which could be real-time sensitive. Since I raised this comment on X86, I think raising it for ARM for discussion would be ok, it might make sense to have consistent API experience for arm/x86 here. Thanks -Jeff [1] https://lore.kernel.org/lkml/CABi2SkWjF2Sicrr71=a6h8xjyf9q9l_nd5fpp0cj2mvb46r...@mail.gmail.com/ > The x86 series also added kselftests to ensure that no spurious SIGSEGV > occurs during signal delivery regardless of which pkey is accessible at > the point where the signal is delivered. This series adapts those > kselftests to allow running them on arm64 (patch 4-5). There is a > dependency on Yury's PKEY_UNRESTRICTED patch [7] for patch 4 > specifically. > > Finally patch 2 is a clean-up following feedback on Joey's series [5]. > > I have tested this series on arm64 and x86_64 (booting and running the > protection_keys and pkey_sighandler_tests mm kselftests). > > - Kevin > > --- > > v2..v3: > * Reordered patches (patch 1 is now the main patch). > * Patch 1: compute por_enable_all with an explicit loop, based on > arch_max_pkey() (suggestion from Dave M). > * Patch 4: improved naming, replaced global pkey reg value with inline > helper, made use of Yury's PKEY_UNRESTRICTED macro [7] (suggestions > from Dave H). > > v2: > https://lore.kernel.org/linux-arm-kernel/20241023150511.3923558-1-kevin.brod...@arm.com/ > > v1..v2: > * In setup_rt_frame(), ensured that POR_EL0 is reset to its original > value if we fail to deliver the signal (addresses Catalin's concern [6]). > * Renamed *unpriv_access* to *user_access* in patch 3 (suggestion from > Dave). > * Made what patch 1-2 do explicit in the commit message body (suggestion > from Dave). > > v1: > https://lore.kernel.org/linux-arm-kernel/20241017133909.3837547-1-kevin.brod...@arm.com/ > > [1] > https://lore.kernel.org/linux-arm-kernel/20240822151113.1479789-1-joey.go...@arm.com/ > [2] > https://lore.kernel.org/lkml/20240802061318.2140081-1-aruna.ramakris...@oracle.com/ > [3] > https://lore.kernel.org/lkml/cabi2skwxnkp2o7ipkp67wkz0-lv33e5brreevtttba6okuf...@mail.gmail.com/ > [4] https://lore.kernel.org/linux-arm-kernel/87plns8owh....@arm.com/ > [5] > https://lore.kernel.org/linux-arm-kernel/20241015114116.GA19334@willie-the-truck/ > [6] https://lore.kernel.org/linux-arm-kernel/zw6d2wavyiwye...@arm.com/ > [7] > https://lore.kernel.org/all/20241028090715.509527-2-yury.khrusta...@arm.com/ > > Cc: a...@linux-foundation.org > Cc: anshuman.khand...@arm.com > Cc: aruna.ramakris...@oracle.com > Cc: broo...@kernel.org > Cc: catalin.mari...@arm.com > Cc: dave.han...@linux.intel.com > Cc: dave.mar...@arm.com > Cc: jef...@chromium.org > Cc: joey.go...@arm.com > Cc: keith.lu...@oracle.com > Cc: pierre.langl...@arm.com > Cc: sh...@kernel.org > Cc: sroett...@google.com > Cc: t...@linutronix.de > Cc: w...@kernel.org > Cc: yury.khrusta...@arm.com > Cc: linux-kselftest@vger.kernel.org > Cc: x...@kernel.org > > Kevin Brodsky (5): > arm64: signal: Improve POR_EL0 handling to avoid uaccess failures > arm64: signal: Remove unnecessary check when saving POE state > arm64: signal: Remove unused macro > selftests/mm: Use generic pkey register manipulation > selftests/mm: Enable pkey_sighandler_tests on arm64 > > arch/arm64/kernel/signal.c | 95 ++++++++++++--- > tools/testing/selftests/mm/Makefile | 8 +- > tools/testing/selftests/mm/pkey-arm64.h | 1 + > tools/testing/selftests/mm/pkey-x86.h | 2 + > .../selftests/mm/pkey_sighandler_tests.c | 115 ++++++++++++++---- > 5 files changed, 176 insertions(+), 45 deletions(-) > > -- > 2.43.0 >
