[PATCH 0/5] Guarded Userspace Access Prevention on Radix

* [PATCH 0/5] Guarded Userspace Access Prevention on Radix
@ 2018-10-26  6:35 Russell Currey
  2018-10-26  6:35 ` [PATCH 1/5] powerpc/64s: Guarded Userspace Access Prevention Russell Currey
                   ` (5 more replies)
  0 siblings, 6 replies; 23+ messages in thread
From: Russell Currey @ 2018-10-26  6:35 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mikey, Russell Currey, npiggin

Guarded Userspace Access Prevention is a security mechanism that prevents
the kernel from being able to read and write userspace addresses outside of
the allowed paths, most commonly copy_{to/from}_user().

At present, the only CPU that supports this is POWER9, and only while using
the Radix MMU.  Privileged reads and writes cannot access user data when
key 0 of the AMR is set.  This is described in the "Radix Tree Translation
Storage Protection" section of the POWER ISA as of version 3.0.

GUAP code sets key 0 of the AMR (thus disabling accesses of user data)
early during boot, and only ever "unlocks" access prior to certain
operations, like copy_{to/from}_user(), futex ops, etc.  Setting this does
not prevent unprivileged access, so userspace can operate fine while access
is locked.

There is a performance impact, although I don't consider it heavy.  Running
a worst-case benchmark of a 1GB copy 1 byte at a time (and thus constant
read(1) write(1) syscalls), I found enabling GUAP to be 3.5% slower than
when disabled.  In most cases, the difference is negligible.  The main
performance impact is the mtspr instruction, which is quite slow.

There are a few caveats with this series that could be improved upon in
future.  Right now there is no saving and restoring of the AMR value -
there is no userspace exploitation of the AMR on Radix in POWER9, but if
this were to change in future, saving and restoring the value would be
necessary.

No attempt to optimise cases of repeated calls - for example, if some
code was repeatedly calling copy_to_user() for small sizes very frequently,
it would be slower than the equivalent of wrapping that code in an unlock
and lock and only having to modify the AMR once.

There are some interesting cases that I've attempted to handle, such as if
the AMR is unlocked (i.e. because a copy_{to_from}_user is in progress)...

    - and an exception is taken, the kernel would then be running with the
    AMR unlocked and freely able to access userspace again.  I am working
    around this by storing a flag in the PACA to indicate if the AMR is
    unlocked (to save a costly SPR read), and if so, locking the AMR in
    the exception entry path and unlocking it on the way out.

    - and gets context switched out, goes into a path that locks the AMR,
    then context switches back, access will be disabled and will fault.
    As a result, I context switch the AMR between tasks as if it was used
    by userspace like hash (which already implements this).

Another consideration is use of the isync instruction.  Without an isync
following the mtspr instruction, there is no guarantee that the change
takes effect.  The issue is that isync is very slow, and so I tried to
avoid them wherever necessary.  In this series, the only place an isync
gets used is after *unlocking* the AMR, because if an access takes place
and access is still prevented, the kernel will fault.

On the flipside, a slight delay in unlocking caused by skipping an isync
potentially allows a small window of vulnerability.  It is my opinion
that this window is practically impossible to exploit, but if someone
thinks otherwise, please do share.

This series is my first attempt at POWER assembly so all feedback is very
welcome.

The official theme song of this series can be found here:
    https://www.youtube.com/watch?v=QjTrnKAcYjE

Russell Currey (5):
  powerpc/64s: Guarded Userspace Access Prevention
  powerpc/futex: GUAP support for futex ops
  powerpc/lib: checksum GUAP support
  powerpc/64s: Disable GUAP with nosmap option
  powerpc/64s: Document that PPC supports nosmap

 .../admin-guide/kernel-parameters.txt         |  2 +-
 arch/powerpc/include/asm/exception-64e.h      |  3 +
 arch/powerpc/include/asm/exception-64s.h      | 19 ++++++-
 arch/powerpc/include/asm/futex.h              |  6 ++
 arch/powerpc/include/asm/mmu.h                |  7 +++
 arch/powerpc/include/asm/paca.h               |  3 +
 arch/powerpc/include/asm/reg.h                |  1 +
 arch/powerpc/include/asm/uaccess.h            | 57 ++++++++++++++++---
 arch/powerpc/kernel/asm-offsets.c             |  1 +
 arch/powerpc/kernel/dt_cpu_ftrs.c             |  4 ++
 arch/powerpc/kernel/entry_64.S                | 17 +++++-
 arch/powerpc/lib/checksum_wrappers.c          |  6 +-
 arch/powerpc/mm/fault.c                       |  9 +++
 arch/powerpc/mm/init_64.c                     | 15 +++++
 arch/powerpc/mm/pgtable-radix.c               |  2 +
 arch/powerpc/mm/pkeys.c                       |  7 ++-
 arch/powerpc/platforms/Kconfig.cputype        | 15 +++++
 17 files changed, 158 insertions(+), 16 deletions(-)

-- 
2.19.1

^ permalink raw reply	[flat|nested] 23+ messages in thread