From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C4F71C0044C for ; Wed, 31 Oct 2018 17:00:58 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 423B620830 for ; Wed, 31 Oct 2018 17:00:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 423B620830 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=c-s.fr Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 42lZNR5N4MzDqmf for ; Thu, 1 Nov 2018 04:00:55 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=c-s.fr Authentication-Results: lists.ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=c-s.fr (client-ip=93.17.236.30; helo=pegase1.c-s.fr; envelope-from=christophe.leroy@c-s.fr; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=c-s.fr Received: from pegase1.c-s.fr (pegase1.c-s.fr [93.17.236.30]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 42lZL53zMXzF2Ds for ; Thu, 1 Nov 2018 03:58:53 +1100 (AEDT) Received: from localhost (mailhub1-int [192.168.12.234]) by localhost (Postfix) with ESMTP id 42lZKz5tftz9ttFb; Wed, 31 Oct 2018 17:58:47 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at c-s.fr Received: from pegase1.c-s.fr ([192.168.12.234]) by localhost (pegase1.c-s.fr [192.168.12.234]) (amavisd-new, port 10024) with ESMTP id uWa1Rp1lX09I; Wed, 31 Oct 2018 17:58:47 +0100 (CET) Received: from vm-hermes.si.c-s.fr (vm-hermes.si.c-s.fr [192.168.25.253]) by pegase1.c-s.fr (Postfix) with ESMTP id 42lZKz5DyCz9ttBW; Wed, 31 Oct 2018 17:58:47 +0100 (CET) Received: by vm-hermes.si.c-s.fr (Postfix, from userid 33) id 272A669F; Wed, 31 Oct 2018 17:58:45 +0100 (CET) Received: from 37.165.103.10 ([37.165.103.10]) by messagerie.si.c-s.fr (Horde Framework) with HTTP; Wed, 31 Oct 2018 17:58:45 +0100 Date: Wed, 31 Oct 2018 17:58:45 +0100 Message-ID: <20181031175845.Horde.o95YpPKYLOaCZUubOS1NRw1@messagerie.si.c-s.fr> From: LEROY Christophe To: Russell Currey Subject: Re: [PATCH 0/5] Guarded Userspace Access Prevention on Radix References: <20181026182916.Horde.8LHYJj4tB_PV5JLEA5Czjw1@messagerie.si.c-s.fr> <0e66ee72293811f580bd02ab4ed7967a4b703d70.camel@russell.cc> In-Reply-To: <0e66ee72293811f580bd02ab4ed7967a4b703d70.camel@russell.cc> User-Agent: Internet Messaging Program (IMP) H5 (6.2.3) Content-Type: text/plain; charset=UTF-8; format=flowed; DelSp=Yes MIME-Version: 1.0 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mikey@neuling.org, linuxppc-dev@lists.ozlabs.org, npiggin@gmail.com Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" Russell Currey a =C3=A9crit=C2=A0: > On Fri, 2018-10-26 at 18:29 +0200, LEROY Christophe wrote: >> Russell Currey a =C3=A9crit : >> >> > 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. >> >> It is not right that only power9 can support that. > > It's true that not only P9 can support it, but there are more > considerations under hash than radix, implementing this for radix is a > first step. I don't know much about hash, but I was talking about the 8xx which is=20= =20 a=20nohash ppc32. I'll see next week if I can do something with it on=20=20 top=20of your serie. > >> >> The 8xx has mmu access protection registers which can serve the >> same >> purpose. Today on the 8xx kernel space is group 0 and user space is >> group 1. Group 0 is set to "page defined access permission" in >> MD_AP >> and MI_AP registers, and group 1 is set to "all accesses done with >> supervisor rights". By setting group 1 to "user and supervisor >> interpretation swapped" we can forbid kernel access to user space >> while still allowing user access to it. Then by simply changing >> group >> 1 mode at dedicated places we can lock/unlock kernel access to user >> space. >> >> Could you implement something as generic as possible having that in >> mind for a future patch ? > > I don't think anything in this series is particularly problematic in > relation to future work for hash. I am interested in doing a hash > implementation in future too. I think we have to look at that carrefuly to avoid uggly ifdefs Christophe > > - Russell > >> >> Christophe >> >> > 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=3DQjTrnKAcYjE >> > >> > 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 >> >>