Re: [PATCH 1/5] powerpc/64s: Kernel Hypervisor Restricted Access Prevention
flat view
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2018-10-17 13:05:19
On Wed, 17 Oct 2018 17:44:19 +1100 Russell Currey [off-list ref] wrote:
Kernel Hypervisor Restricted Access Prevention (KHRAP) utilises a feature
of the Radix MMU which disallows read and write access to userspace
addresses. By utilising this, the kernel is prevented from accessing
user data from outside of trusted paths that perform proper safety checks,
such as copy_{to/from}_user() and friends.
Userspace access is disabled from early boot and is only enabled when:
- exiting the kernel and entering userspace
- performing an operation like copy_{to/from}_user()
- context switching to a process that has access enabled
and similarly, access is disabled again when exiting userspace and entering
the kernel.
This feature has a slight performance impact which I roughly measured to be
4% slower (performing 1GB of 1 byte read()/write() syscalls), and is gated
behind the CONFIG_PPC_RADIX_KHRAP option for performance-critical builds.
This feature can be tested by using the lkdtm driver (CONFIG_LKDTM=y) and
performing the following:
echo ACCESS_USERSPACE > [debugfs]/provoke-crash/DIRECT
if enabled, this should send SIGSEGV to the thread.
Signed-off-by: Russell Currey <redacted>
---
More detailed benchmarks soon, there's more optimisations here as well.Nice, this turned out to be a lot neater than I feared! Good stuff.
quoted hunk ↗ jump to hunk
@@ -240,6 +240,22 @@ BEGIN_FTR_SECTION_NESTED(941) \ mtspr SPRN_PPR,ra; \ END_FTR_SECTION_NESTED(CPU_FTR_HAS_PPR,CPU_FTR_HAS_PPR,941) +#define LOCK_AMR(reg) \ +BEGIN_MMU_FTR_SECTION_NESTED(69) \ + LOAD_REG_IMMEDIATE(reg,AMR_LOCKED); \ + isync; \ + mtspr SPRN_AMR,reg; \ + isync; \ +END_MMU_FTR_SECTION_NESTED(MMU_FTR_RADIX_KHRAP,MMU_FTR_RADIX_KHRAP,69) + +#define UNLOCK_AMR(reg) \ +BEGIN_MMU_FTR_SECTION_NESTED(420) \ + li reg,0; \ + isync; \ + mtspr SPRN_AMR,reg; \ + isync; \ +END_MMU_FTR_SECTION_NESTED(MMU_FTR_RADIX_KHRAP,MMU_FTR_RADIX_KHRAP,420)
I wonder if you can skip the first isync on the way in and the second isync on the way out because the interrupt and return should be context synchronizing. Might not make a difference though. What do you think about making the name match the C code a bit more. Like AMR_LOCK_USER_ACCESS()? Thanks, Nick