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=-8.3 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 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 98C45CA9EAF for ; Wed, 30 Oct 2019 08:01:54 +0000 (UTC) Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.kernel.org (Postfix) with SMTP id B31FA20578 for ; Wed, 30 Oct 2019 08:01:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=c-s.fr header.i=@c-s.fr header.b="V8hqm9Qb" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B31FA20578 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=kernel-hardening-return-17164-kernel-hardening=archiver.kernel.org@lists.openwall.com Received: (qmail 15886 invoked by uid 550); 30 Oct 2019 08:01:46 -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: Received: (qmail 15851 invoked from network); 30 Oct 2019 08:01:46 -0000 Authentication-Results: localhost; dkim=pass reason="1024-bit key; insecure key" header.d=c-s.fr header.i=@c-s.fr header.b=V8hqm9Qb; dkim-adsp=pass; dkim-atps=neutral X-Virus-Scanned: Debian amavisd-new at c-s.fr DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=c-s.fr; s=mail; t=1572422493; bh=dgUPwu14dlfDLkwnSUPxtAKJre3nQyA0++wcWBZvbX8=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=V8hqm9Qb5quUNx5SfqiyhYBC8DUGvl/rHB57Fvah1n6VF+h5k5wNR5YaLNBFQAsDZ AFmXjCHJmLvPzmorL01ywkeoBfIaXTumvXK9uVowarBGe90t2AkShv0fNI6DWUUEgn Y1LeHhoFrbKXAsFfy5eJ6+qlmZMo5btt678iTm1A= X-Virus-Scanned: amavisd-new at c-s.fr Subject: Re: [PATCH v5 1/5] powerpc/mm: Implement set_memory() routines To: Russell Currey , linuxppc-dev@lists.ozlabs.org Cc: joel@jms.id.au, mpe@ellerman.id.au, ajd@linux.ibm.com, dja@axtens.net, npiggin@gmail.com, kernel-hardening@lists.openwall.com References: <20191030073111.140493-1-ruscur@russell.cc> <20191030073111.140493-2-ruscur@russell.cc> From: Christophe Leroy Message-ID: <5cea9974-9bef-712c-6e7b-b3c5fa7e0702@c-s.fr> Date: Wed, 30 Oct 2019 09:01:33 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20191030073111.140493-2-ruscur@russell.cc> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: fr Content-Transfer-Encoding: 8bit Le 30/10/2019 à 08:31, Russell Currey a écrit : > The set_memory_{ro/rw/nx/x}() functions are required for STRICT_MODULE_RWX, > and are generally useful primitives to have. This implementation is > designed to be completely generic across powerpc's many MMUs. > > It's possible that this could be optimised to be faster for specific > MMUs, but the focus is on having a generic and safe implementation for > now. > > This implementation does not handle cases where the caller is attempting > to change the mapping of the page it is executing from, or if another > CPU is concurrently using the page being altered. These cases likely > shouldn't happen, but a more complex implementation with MMU-specific code > could safely handle them, so that is left as a TODO for now. > > Signed-off-by: Russell Currey > --- > arch/powerpc/Kconfig | 1 + > arch/powerpc/include/asm/set_memory.h | 32 +++++++++++ > arch/powerpc/mm/Makefile | 1 + > arch/powerpc/mm/pageattr.c | 77 +++++++++++++++++++++++++++ > 4 files changed, 111 insertions(+) > create mode 100644 arch/powerpc/include/asm/set_memory.h > create mode 100644 arch/powerpc/mm/pageattr.c > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index 3e56c9c2f16e..8f7005f0d097 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -133,6 +133,7 @@ config PPC > select ARCH_HAS_PTE_SPECIAL > select ARCH_HAS_MEMBARRIER_CALLBACKS > select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64 > + select ARCH_HAS_SET_MEMORY > select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !RELOCATABLE && !HIBERNATION) > select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST > select ARCH_HAS_UACCESS_FLUSHCACHE > diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h > new file mode 100644 > index 000000000000..5230ddb2fefd > --- /dev/null > +++ b/arch/powerpc/include/asm/set_memory.h > @@ -0,0 +1,32 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ASM_POWERPC_SET_MEMORY_H > +#define _ASM_POWERPC_SET_MEMORY_H > + > +#define SET_MEMORY_RO 1 > +#define SET_MEMORY_RW 2 > +#define SET_MEMORY_NX 3 > +#define SET_MEMORY_X 4 > + > +int change_memory_attr(unsigned long addr, int numpages, int action); > + > +static inline int set_memory_ro(unsigned long addr, int numpages) > +{ > + return change_memory_attr(addr, numpages, SET_MEMORY_RO); > +} > + > +static inline int set_memory_rw(unsigned long addr, int numpages) > +{ > + return change_memory_attr(addr, numpages, SET_MEMORY_RW); > +} > + > +static inline int set_memory_nx(unsigned long addr, int numpages) > +{ > + return change_memory_attr(addr, numpages, SET_MEMORY_NX); > +} > + > +static inline int set_memory_x(unsigned long addr, int numpages) > +{ > + return change_memory_attr(addr, numpages, SET_MEMORY_X); > +} > + > +#endif > diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile > index 5e147986400d..d0a0bcbc9289 100644 > --- a/arch/powerpc/mm/Makefile > +++ b/arch/powerpc/mm/Makefile > @@ -20,3 +20,4 @@ obj-$(CONFIG_HIGHMEM) += highmem.o > obj-$(CONFIG_PPC_COPRO_BASE) += copro_fault.o > obj-$(CONFIG_PPC_PTDUMP) += ptdump/ > obj-$(CONFIG_KASAN) += kasan/ > +obj-$(CONFIG_ARCH_HAS_SET_MEMORY) += pageattr.o > diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c > new file mode 100644 > index 000000000000..aedd79173a44 > --- /dev/null > +++ b/arch/powerpc/mm/pageattr.c > @@ -0,0 +1,77 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* > + * MMU-generic set_memory implementation for powerpc > + * > + * Copyright 2019, IBM Corporation. > + */ > + > +#include > +#include > + > +#include > +#include > +#include > + > + > +/* > + * Updates the attributes of a page in three steps: > + * > + * 1. invalidate the page table entry > + * 2. flush the TLB > + * 3. install the new entry with the updated attributes > + * > + * This is unsafe if the caller is attempting to change the mapping of the > + * page it is executing from, or if another CPU is concurrently using the > + * page being altered. > + * > + * TODO make the implementation resistant to this. > + */ > +static int change_page_attr(pte_t *ptep, unsigned long addr, void *data) I don't like too much the way you are making this function more complex and less readable with local var, goto, etc ... You could just keep the v4 version of change_page_attr(), rename it __change_page_attr(), then add: static int change_page_attr(pte_t *ptep, unsigned long addr, void *data) { int ret; spin_lock(&init_mm.page_table_lock); ret = __change_page_attr(ptep, addr, data); spin_unlock(&init_mm.page_table_lock); return ret; } Christophe > +{ > + int action = *((int *)data); > + pte_t pte_val; > + int ret = 0; > + > + spin_lock(&init_mm.page_table_lock); > + > + // invalidate the PTE so it's safe to modify > + pte_val = ptep_get_and_clear(&init_mm, addr, ptep); > + flush_tlb_kernel_range(addr, addr + PAGE_SIZE); > + > + // modify the PTE bits as desired, then apply > + switch (action) { > + case SET_MEMORY_RO: > + pte_val = pte_wrprotect(pte_val); > + break; > + case SET_MEMORY_RW: > + pte_val = pte_mkwrite(pte_val); > + break; > + case SET_MEMORY_NX: > + pte_val = pte_exprotect(pte_val); > + break; > + case SET_MEMORY_X: > + pte_val = pte_mkexec(pte_val); > + break; > + default: > + WARN_ON(true); > + ret = -EINVAL; > + goto out; > + } > + > + set_pte_at(&init_mm, addr, ptep, pte_val); > +out: > + spin_unlock(&init_mm.page_table_lock); > + return ret; > +} > + > +int change_memory_attr(unsigned long addr, int numpages, int action) > +{ > + unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE); > + unsigned long size = numpages * PAGE_SIZE; > + > + if (!numpages) > + return 0; > + > + return apply_to_page_range(&init_mm, start, size, change_page_attr, &action); > +} >