From patchwork Thu Oct 27 14:54:50 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Catalin Marinas X-Patchwork-Id: 9399671 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C404F60231 for ; Thu, 27 Oct 2016 14:55:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B03C52A30D for ; Thu, 27 Oct 2016 14:55:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A50332A30F; Thu, 27 Oct 2016 14:55:12 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id 8235F2A30D for ; Thu, 27 Oct 2016 14:55:08 +0000 (UTC) Received: (qmail 24460 invoked by uid 550); 27 Oct 2016 14:55:07 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: kernel-hardening@lists.openwall.com Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 24439 invoked from network); 27 Oct 2016 14:55:06 -0000 Date: Thu, 27 Oct 2016 15:54:50 +0100 From: Catalin Marinas To: Kees Cook Cc: Sami Tolvanen , Ard Biesheuvel , "kernel-hardening@lists.openwall.com" , Will Deacon , AKASHI Takahiro , James Morse , andre.przywara@arm.com, "Suzuki K. Poulose" , "linux-arm-kernel@lists.infradead.org" Message-ID: <20161027145450.GB3762@e104818-lin.cambridge.arm.com> References: <1473788797-10879-1-git-send-email-catalin.marinas@arm.com> <20160915162044.GB19214@leverpostej> <20160929224452.GA71670@samitolvanen.mtv.corp.google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [kernel-hardening] Re: [PATCH v3 0/7] arm64: Privileged Access Never using TTBR0_EL1 switching X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Sep 30, 2016 at 11:42:02AM -0700, Kees Cook wrote: > On Thu, Sep 29, 2016 at 3:44 PM, Sami Tolvanen wrote: > > On Thu, Sep 15, 2016 at 05:20:45PM +0100, Mark Rutland wrote: > >> Likewise, how do we handle __flush_cache_user_range and > >> flush_icache_range? Some callers (e.g. __do_compat_cache_op) pass in > >> __user addresses. > > > > Also EXEC_USERSPACE in lkdtm passes a user space address to flush_icache_range > > and causes the process to hang when I tested these patches on HiKey. > > > > Adding uaccess_{enable,disable}_not_uao to __flush_cache_user_range appears to > > fix the problem. > > I had a thought just now on this: is lkdtm maybe doing the wrong thing > here? i.e. should lkdtm be the one do to the uaccess_en/disable > instead of flush_icache_range() itself, since it's the one abusing the > API? (preparing the v4 series) I think lkdtm is using the API incorrectly here. The documentation for flush_icache_range() (Documentation/cachetlb.txt) states that it is to be used on kernel addresses. Even with uaccess_enable/disable in lkdtm, faults on user space can still happen and the flush_icache_range() function must be able to handle them. It happens to work on arm64 because of the fall through __flush_cache_user_range() but that's not guaranteed on other architectures. A potential solution is to use access_process_vm() and let the arch code handle the cache maintenance automatically: ---------------------8<-------------------------------- From fcbb7c9c30daf9bfc2a215ec10dba79c109ab835 Mon Sep 17 00:00:00 2001 From: Catalin Marinas Date: Thu, 27 Oct 2016 15:47:20 +0100 Subject: [PATCH] lkdtm: Do not use flush_icache_range() on user addresses The flush_icache_range() API is meant to be used on kernel addresses only as it may not have the infrastructure (exception entries) to handle user memory faults. The lkdtm execute_user_location() function tests the kernel execution of user space addresses by mmap'ing an anonymous page, copying some code together with cache maintenance and attempting to run it. However, the cache maintenance step may fail because of the incorrect API usage described above. The patch changes lkdtm to use access_process_vm() for copying the code into user space which would take care of the necessary cache maintenance. Signed-off-by: Catalin Marinas --- drivers/misc/lkdtm_perms.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/misc/lkdtm_perms.c b/drivers/misc/lkdtm_perms.c index 45f1c0f96612..c7635a79341f 100644 --- a/drivers/misc/lkdtm_perms.c +++ b/drivers/misc/lkdtm_perms.c @@ -60,15 +60,18 @@ static noinline void execute_location(void *dst, bool write) static void execute_user_location(void *dst) { + int copied; + /* Intentionally crossing kernel/user memory boundary. */ void (*func)(void) = dst; pr_info("attempting ok execution at %p\n", do_nothing); do_nothing(); - if (copy_to_user((void __user *)dst, do_nothing, EXEC_SIZE)) + copied = access_process_vm(current, (unsigned long)dst, do_nothing, + EXEC_SIZE, FOLL_WRITE); + if (copied < EXEC_SIZE) return; - flush_icache_range((unsigned long)dst, (unsigned long)dst + EXEC_SIZE); pr_info("attempting bad execution at %p\n", func); func(); }