+Kees and Jorge and Jann On Tue, Oct 29, 2024 at 7:45 AM Kevin Brodsky <kevin.brodsky@xxxxxxx> 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_Nd5FPp0CJ2mvB46Rrrg@xxxxxxxxxxxxxx/ > 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.brodsky@xxxxxxx/ > > 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.brodsky@xxxxxxx/ > > [1] https://lore.kernel.org/linux-arm-kernel/20240822151113.1479789-1-joey.gouly@xxxxxxx/ > [2] https://lore.kernel.org/lkml/20240802061318.2140081-1-aruna.ramakrishna@xxxxxxxxxx/ > [3] https://lore.kernel.org/lkml/CABi2SkWxNkP2O7ipkP67WKz0-LV33e5brReevTTtba6oKUfHRw@xxxxxxxxxxxxxx/ > [4] https://lore.kernel.org/linux-arm-kernel/87plns8owh.fsf@xxxxxxx/ > [5] https://lore.kernel.org/linux-arm-kernel/20241015114116.GA19334@willie-the-truck/ > [6] https://lore.kernel.org/linux-arm-kernel/Zw6D2waVyIwYE7wd@xxxxxxx/ > [7] https://lore.kernel.org/all/20241028090715.509527-2-yury.khrustalev@xxxxxxx/ > > Cc: akpm@xxxxxxxxxxxxxxxxxxxx > Cc: anshuman.khandual@xxxxxxx > Cc: aruna.ramakrishna@xxxxxxxxxx > Cc: broonie@xxxxxxxxxx > Cc: catalin.marinas@xxxxxxx > Cc: dave.hansen@xxxxxxxxxxxxxxx > Cc: Dave.Martin@xxxxxxx > Cc: jeffxu@xxxxxxxxxxxx > Cc: joey.gouly@xxxxxxx > Cc: keith.lucas@xxxxxxxxxx > Cc: pierre.langlois@xxxxxxx > Cc: shuah@xxxxxxxxxx > Cc: sroettger@xxxxxxxxxx > Cc: tglx@xxxxxxxxxxxxx > Cc: will@xxxxxxxxxx > Cc: yury.khrustalev@xxxxxxx > Cc: linux-kselftest@xxxxxxxxxxxxxxx > Cc: x86@xxxxxxxxxx > > 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 >
