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=-6.9 required=3.0 tests=DKIMWL_WL_HIGH,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 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 A4566CA9EAE for ; Tue, 29 Oct 2019 23:19:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6A8D32087E for ; Tue, 29 Oct 2019 23:19:55 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="ljpbXbSC" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726425AbfJ2XTy (ORCPT ); Tue, 29 Oct 2019 19:19:54 -0400 Received: from mail-pl1-f194.google.com ([209.85.214.194]:33177 "EHLO mail-pl1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726331AbfJ2XTx (ORCPT ); Tue, 29 Oct 2019 19:19:53 -0400 Received: by mail-pl1-f194.google.com with SMTP id y8so66975plk.0 for ; Tue, 29 Oct 2019 16:19:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=YaJtgLYKb0l4FyZ4jUBHCczpuWveP563IeeZfWc0m7k=; b=ljpbXbSCQr3faVn4lcJ2hNpbtNEtMXu99g0vus9AnDxFazy8h/FibLkL2N5dPTndSO Svau97KKRz8S4/mdymps+zi6jIgxLv+30soShO3eyIJszJ2mOkKx2q0gPesRu3Gadt6M J4Ghoo5SGJFENRAHtiAV7eKJVlA3ieoqc3EUc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=YaJtgLYKb0l4FyZ4jUBHCczpuWveP563IeeZfWc0m7k=; b=coqz4MVanScs7ub6HJyRUqfnkDyK0eCOwfleo6qRgtlMwE/iaI/d43uQ4aHhzJPoZ7 w/5/j0vHgC7oc2FMLl/3Nu335w0P/G2J8slc59N1R+ANns9/qSGIvFszRvSwv/keQvqU wjyq4F3dVuRtYOMZ/+uOxlEcaCixqFO9fYroREYnzFW0OyBfkAbZneVQMHn/+itJVg7Z 5IhWxCqYMyw+pqzE4AuSPGc9k3+M0sB6qkWi3sjOIqiXHIXuFF9TVitlav0+oqZnNvwa Ohr8Bv8YRipJuI4NYJSCr3gBmC989y6V9aR/E6hsxue7r14Kl9OZpW034u8//4DxVYJ0 YUIA== X-Gm-Message-State: APjAAAUsIsiabAf/tFgj5NFtKHnSJH7MaWeMweMPOeiLRnJJ8iUDpqvd frsS4ycAD3bOrOfBtgRZb9ZR4Sd/jlU= X-Google-Smtp-Source: APXvYqzWIvlFIF+G6hAYMTtuyrvwFzMMaImF7+nXc9spFzUJ/+qexBUliEgWpaVwDQLWqyzr2bSMqA== X-Received: by 2002:a17:902:b7c2:: with SMTP id v2mr1270214plz.202.1572391192503; Tue, 29 Oct 2019 16:19:52 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id f185sm177613pfb.183.2019.10.29.16.19.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Oct 2019 16:19:51 -0700 (PDT) Date: Tue, 29 Oct 2019 16:19:50 -0700 From: Kees Cook To: Dave Martin Cc: linux-kernel@vger.kernel.org, Andrew Jones , Arnd Bergmann , Catalin Marinas , Eugene Syromiatnikov , Florian Weimer , "H.J. Lu" , Jann Horn , Kristina =?utf-8?Q?Mart=C5=A1enko?= , Marc Zyngier , Mark Brown , Paul Elliott , Peter Zijlstra , Richard Henderson , Sudakshina Das , Szabolcs Nagy , Thomas Gleixner , Will Deacon , Yu-cheng Yu , Amit Kachhap , Vincenzo Frascino , linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 06/12] elf: Allow arch to tweak initial mmap prot flags Message-ID: <201910291618.C28E737@keescook> References: <1571419545-20401-1-git-send-email-Dave.Martin@arm.com> <1571419545-20401-7-git-send-email-Dave.Martin@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1571419545-20401-7-git-send-email-Dave.Martin@arm.com> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 18, 2019 at 06:25:39PM +0100, Dave Martin wrote: > An arch may want to tweak the mmap prot flags for an > ELFexecutable's initial mappings. For example, arm64 is going to > need to add PROT_BTI for executable pages in an ELF process whose > executable is marked as using Branch Target Identification (an > ARMv8.5-A control flow integrity feature). > > So that this can be done in a generic way, add a hook > arch_elf_adjust_prot() to modify the prot flags as desired: arches > can select CONFIG_HAVE_ELF_PROT and implement their own backend > where necessary. > > By default, leave the prot flags unchanged. > > Signed-off-by: Dave Martin > --- > fs/Kconfig.binfmt | 3 +++ > fs/binfmt_elf.c | 18 ++++++++++++------ > include/linux/elf.h | 12 ++++++++++++ > 3 files changed, 27 insertions(+), 6 deletions(-) > > diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt > index d2cfe07..2358368 100644 > --- a/fs/Kconfig.binfmt > +++ b/fs/Kconfig.binfmt > @@ -36,6 +36,9 @@ config COMPAT_BINFMT_ELF > config ARCH_BINFMT_ELF_STATE > bool > > +config ARCH_HAVE_ELF_PROT > + bool > + > config ARCH_USE_GNU_PROPERTY > bool > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c > index ae345f6..dbfab2e 100644 > --- a/fs/binfmt_elf.c > +++ b/fs/binfmt_elf.c > @@ -531,7 +531,8 @@ static inline int arch_check_elf(struct elfhdr *ehdr, bool has_interp, > > #endif /* !CONFIG_ARCH_BINFMT_ELF_STATE */ > > -static inline int make_prot(u32 p_flags) > +static inline int make_prot(u32 p_flags, struct arch_elf_state *arch_state, > + bool has_interp, bool is_interp) > { > int prot = 0; > > @@ -541,7 +542,8 @@ static inline int make_prot(u32 p_flags) > prot |= PROT_WRITE; > if (p_flags & PF_X) > prot |= PROT_EXEC; > - return prot; > + > + return arch_elf_adjust_prot(prot, arch_state, has_interp, is_interp); > } Random thought: there is already an 'exec-state' structure: bprm. I wonder if arch_state should be attached there instead of passed around here? It'd require almost the same amount of changes, though, so my idea might not gain much in the diffstat, but maybe it's better organizationally? I'm not sure. Otherwise, this all looks fine to me. Reviewed-by: Kees Cook -Kees > > /* This is much more generalized than the library routine read function, > @@ -551,7 +553,8 @@ static inline int make_prot(u32 p_flags) > > static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, > struct file *interpreter, unsigned long *interp_map_addr, > - unsigned long no_base, struct elf_phdr *interp_elf_phdata) > + unsigned long no_base, struct elf_phdr *interp_elf_phdata, > + struct arch_elf_state *arch_state) > { > struct elf_phdr *eppnt; > unsigned long load_addr = 0; > @@ -583,7 +586,8 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, > for (i = 0; i < interp_elf_ex->e_phnum; i++, eppnt++) { > if (eppnt->p_type == PT_LOAD) { > int elf_type = MAP_PRIVATE | MAP_DENYWRITE; > - int elf_prot = make_prot(eppnt->p_flags); > + int elf_prot = make_prot(eppnt->p_flags, arch_state, > + true, true); > unsigned long vaddr = 0; > unsigned long k, map_addr; > > @@ -1040,7 +1044,8 @@ static int load_elf_binary(struct linux_binprm *bprm) > } > } > > - elf_prot = make_prot(elf_ppnt->p_flags); > + elf_prot = make_prot(elf_ppnt->p_flags, &arch_state, > + !!interpreter, false); > > elf_flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE; > > @@ -1186,7 +1191,8 @@ static int load_elf_binary(struct linux_binprm *bprm) > elf_entry = load_elf_interp(&loc->interp_elf_ex, > interpreter, > &interp_map_addr, > - load_bias, interp_elf_phdata); > + load_bias, interp_elf_phdata, > + &arch_state); > if (!IS_ERR((void *)elf_entry)) { > /* > * load_elf_interp() returns relocation > diff --git a/include/linux/elf.h b/include/linux/elf.h > index 7bdc6da..1b6e895 100644 > --- a/include/linux/elf.h > +++ b/include/linux/elf.h > @@ -83,4 +83,16 @@ extern int arch_parse_elf_property(u32 type, const void *data, size_t datasz, > bool compat, struct arch_elf_state *arch); > #endif > > +#ifdef CONFIG_ARCH_HAVE_ELF_PROT > +int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state, > + bool has_interp, bool is_interp); > +#else > +static inline int arch_elf_adjust_prot(int prot, > + const struct arch_elf_state *state, > + bool has_interp, bool is_interp) > +{ > + return prot; > +} > +#endif > + > #endif /* _LINUX_ELF_H */ > -- > 2.1.4 > -- Kees Cook From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kees Cook Subject: Re: [PATCH v3 06/12] elf: Allow arch to tweak initial mmap prot flags Date: Tue, 29 Oct 2019 16:19:50 -0700 Message-ID: <201910291618.C28E737@keescook> References: <1571419545-20401-1-git-send-email-Dave.Martin@arm.com> <1571419545-20401-7-git-send-email-Dave.Martin@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1571419545-20401-7-git-send-email-Dave.Martin@arm.com> Sender: linux-kernel-owner@vger.kernel.org To: Dave Martin Cc: linux-kernel@vger.kernel.org, Andrew Jones , Arnd Bergmann , Catalin Marinas , Eugene Syromiatnikov , Florian Weimer , "H.J. Lu" , Jann Horn , Kristina =?utf-8?Q?Mart=C5=A1enko?= , Marc Zyngier , Mark Brown , Paul Elliott , Peter Zijlstra , Richard Henderson , Sudakshina Das , Szabolcs Nagy , Thomas Gleixner , Will Deacon , Yu-cheng Yu , Amit Kachhap , Vincenzo List-Id: linux-arch.vger.kernel.org On Fri, Oct 18, 2019 at 06:25:39PM +0100, Dave Martin wrote: > An arch may want to tweak the mmap prot flags for an > ELFexecutable's initial mappings. For example, arm64 is going to > need to add PROT_BTI for executable pages in an ELF process whose > executable is marked as using Branch Target Identification (an > ARMv8.5-A control flow integrity feature). > > So that this can be done in a generic way, add a hook > arch_elf_adjust_prot() to modify the prot flags as desired: arches > can select CONFIG_HAVE_ELF_PROT and implement their own backend > where necessary. > > By default, leave the prot flags unchanged. > > Signed-off-by: Dave Martin > --- > fs/Kconfig.binfmt | 3 +++ > fs/binfmt_elf.c | 18 ++++++++++++------ > include/linux/elf.h | 12 ++++++++++++ > 3 files changed, 27 insertions(+), 6 deletions(-) > > diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt > index d2cfe07..2358368 100644 > --- a/fs/Kconfig.binfmt > +++ b/fs/Kconfig.binfmt > @@ -36,6 +36,9 @@ config COMPAT_BINFMT_ELF > config ARCH_BINFMT_ELF_STATE > bool > > +config ARCH_HAVE_ELF_PROT > + bool > + > config ARCH_USE_GNU_PROPERTY > bool > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c > index ae345f6..dbfab2e 100644 > --- a/fs/binfmt_elf.c > +++ b/fs/binfmt_elf.c > @@ -531,7 +531,8 @@ static inline int arch_check_elf(struct elfhdr *ehdr, bool has_interp, > > #endif /* !CONFIG_ARCH_BINFMT_ELF_STATE */ > > -static inline int make_prot(u32 p_flags) > +static inline int make_prot(u32 p_flags, struct arch_elf_state *arch_state, > + bool has_interp, bool is_interp) > { > int prot = 0; > > @@ -541,7 +542,8 @@ static inline int make_prot(u32 p_flags) > prot |= PROT_WRITE; > if (p_flags & PF_X) > prot |= PROT_EXEC; > - return prot; > + > + return arch_elf_adjust_prot(prot, arch_state, has_interp, is_interp); > } Random thought: there is already an 'exec-state' structure: bprm. I wonder if arch_state should be attached there instead of passed around here? It'd require almost the same amount of changes, though, so my idea might not gain much in the diffstat, but maybe it's better organizationally? I'm not sure. Otherwise, this all looks fine to me. Reviewed-by: Kees Cook -Kees > > /* This is much more generalized than the library routine read function, > @@ -551,7 +553,8 @@ static inline int make_prot(u32 p_flags) > > static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, > struct file *interpreter, unsigned long *interp_map_addr, > - unsigned long no_base, struct elf_phdr *interp_elf_phdata) > + unsigned long no_base, struct elf_phdr *interp_elf_phdata, > + struct arch_elf_state *arch_state) > { > struct elf_phdr *eppnt; > unsigned long load_addr = 0; > @@ -583,7 +586,8 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, > for (i = 0; i < interp_elf_ex->e_phnum; i++, eppnt++) { > if (eppnt->p_type == PT_LOAD) { > int elf_type = MAP_PRIVATE | MAP_DENYWRITE; > - int elf_prot = make_prot(eppnt->p_flags); > + int elf_prot = make_prot(eppnt->p_flags, arch_state, > + true, true); > unsigned long vaddr = 0; > unsigned long k, map_addr; > > @@ -1040,7 +1044,8 @@ static int load_elf_binary(struct linux_binprm *bprm) > } > } > > - elf_prot = make_prot(elf_ppnt->p_flags); > + elf_prot = make_prot(elf_ppnt->p_flags, &arch_state, > + !!interpreter, false); > > elf_flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE; > > @@ -1186,7 +1191,8 @@ static int load_elf_binary(struct linux_binprm *bprm) > elf_entry = load_elf_interp(&loc->interp_elf_ex, > interpreter, > &interp_map_addr, > - load_bias, interp_elf_phdata); > + load_bias, interp_elf_phdata, > + &arch_state); > if (!IS_ERR((void *)elf_entry)) { > /* > * load_elf_interp() returns relocation > diff --git a/include/linux/elf.h b/include/linux/elf.h > index 7bdc6da..1b6e895 100644 > --- a/include/linux/elf.h > +++ b/include/linux/elf.h > @@ -83,4 +83,16 @@ extern int arch_parse_elf_property(u32 type, const void *data, size_t datasz, > bool compat, struct arch_elf_state *arch); > #endif > > +#ifdef CONFIG_ARCH_HAVE_ELF_PROT > +int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state, > + bool has_interp, bool is_interp); > +#else > +static inline int arch_elf_adjust_prot(int prot, > + const struct arch_elf_state *state, > + bool has_interp, bool is_interp) > +{ > + return prot; > +} > +#endif > + > #endif /* _LINUX_ELF_H */ > -- > 2.1.4 > -- Kees Cook 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=-6.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 8D4B4CA9EAE for ; Tue, 29 Oct 2019 23:20:18 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 625E82086A for ; Tue, 29 Oct 2019 23:20:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="tzSkurgA"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="ljpbXbSC" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 625E82086A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=mFp+q21jGD796O8xpDMzRYdzPINv1jllPQjZo+JA9u4=; b=tzSkurgAx0SGmi g+JlxGNnJFx7vNdvJD0g74Vb7K7DVZ/9sEyWZVnv+4uXkphpIys8UC69qQmHvdfvL1t6rdiSzop+w 4m2hVYGOabg1pwd8086DhBWN548Pb5oq62bMN6KT7gRuaBL63blWxKdq0twYRa+g6GFhHpEwSUSNx b9KtOB84Z1bwBOEIFXwlJC9UCAODHh7zCpMtKp6GV4bq2eCfV5PrM0YXOAuEwXZ8+En3U5cV5FMj1 MoxBjBt4xAciWlFYODu3lrEzWkSayQTAi22DFXo2lSgk9Fo+r3EGEERBmTru+j+bPQb+MIEbX179z PNyCr6vvkaPKM/09O5Ww==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iPamf-0000LZ-M1; Tue, 29 Oct 2019 23:20:17 +0000 Received: from mail-pl1-x643.google.com ([2607:f8b0:4864:20::643]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1iPamH-0007F0-Uz for linux-arm-kernel@lists.infradead.org; Tue, 29 Oct 2019 23:19:55 +0000 Received: by mail-pl1-x643.google.com with SMTP id q21so39630plr.13 for ; Tue, 29 Oct 2019 16:19:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=YaJtgLYKb0l4FyZ4jUBHCczpuWveP563IeeZfWc0m7k=; b=ljpbXbSCQr3faVn4lcJ2hNpbtNEtMXu99g0vus9AnDxFazy8h/FibLkL2N5dPTndSO Svau97KKRz8S4/mdymps+zi6jIgxLv+30soShO3eyIJszJ2mOkKx2q0gPesRu3Gadt6M J4Ghoo5SGJFENRAHtiAV7eKJVlA3ieoqc3EUc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=YaJtgLYKb0l4FyZ4jUBHCczpuWveP563IeeZfWc0m7k=; b=gQ0uWwaYHgFP/hlqPE1y9pTnP44xA3WgGch73dOjX3JwBwa1oK68yYTHW6TVhkZx9F 8Id7e2xzkkPBj5lySBzlufGlLB0RetwG8eEPUs3EgwNMAtYIDg4PuX4Ig9viUDcouRI5 5WEaNxrtnuH/lLFFpgwIEi4ECRBpdSiE0frC3CzcvyLrJnPM5T+8XKqHeSEPL45rHeJH Xve7cjMsLLWHIEs2AodJ2nR95Vh74eAIeynzYByMVBgcpgPEPkFcvaniZ/2wTNl5UULf RSutrsdgnCrsh4J+opqbZHELe4Wl4bekCXNXx0xUIRRkAYFCvdWqF6pM38NZlXBB7T9+ Y0tw== X-Gm-Message-State: APjAAAXUURrLQFdjK1AVOaubba4JPAYpsT3OsqlQyAVDX9aHjbB6xJmo w0ykvTBoDuGTt/6ho/29+45sZQ== X-Google-Smtp-Source: APXvYqzWIvlFIF+G6hAYMTtuyrvwFzMMaImF7+nXc9spFzUJ/+qexBUliEgWpaVwDQLWqyzr2bSMqA== X-Received: by 2002:a17:902:b7c2:: with SMTP id v2mr1270214plz.202.1572391192503; Tue, 29 Oct 2019 16:19:52 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id f185sm177613pfb.183.2019.10.29.16.19.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Oct 2019 16:19:51 -0700 (PDT) Date: Tue, 29 Oct 2019 16:19:50 -0700 From: Kees Cook To: Dave Martin Subject: Re: [PATCH v3 06/12] elf: Allow arch to tweak initial mmap prot flags Message-ID: <201910291618.C28E737@keescook> References: <1571419545-20401-1-git-send-email-Dave.Martin@arm.com> <1571419545-20401-7-git-send-email-Dave.Martin@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1571419545-20401-7-git-send-email-Dave.Martin@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191029_161954_108330_9163C809 X-CRM114-Status: GOOD ( 22.51 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paul Elliott , Peter Zijlstra , Catalin Marinas , Yu-cheng Yu , Amit Kachhap , Vincenzo Frascino , Will Deacon , linux-arch@vger.kernel.org, Marc Zyngier , Eugene Syromiatnikov , Szabolcs Nagy , "H.J. Lu" , Andrew Jones , Arnd Bergmann , Jann Horn , Richard Henderson , Kristina =?utf-8?Q?Mart=C5=A1enko?= , Mark Brown , Thomas Gleixner , linux-arm-kernel@lists.infradead.org, Florian Weimer , linux-kernel@vger.kernel.org, Sudakshina Das Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Oct 18, 2019 at 06:25:39PM +0100, Dave Martin wrote: > An arch may want to tweak the mmap prot flags for an > ELFexecutable's initial mappings. For example, arm64 is going to > need to add PROT_BTI for executable pages in an ELF process whose > executable is marked as using Branch Target Identification (an > ARMv8.5-A control flow integrity feature). > > So that this can be done in a generic way, add a hook > arch_elf_adjust_prot() to modify the prot flags as desired: arches > can select CONFIG_HAVE_ELF_PROT and implement their own backend > where necessary. > > By default, leave the prot flags unchanged. > > Signed-off-by: Dave Martin > --- > fs/Kconfig.binfmt | 3 +++ > fs/binfmt_elf.c | 18 ++++++++++++------ > include/linux/elf.h | 12 ++++++++++++ > 3 files changed, 27 insertions(+), 6 deletions(-) > > diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt > index d2cfe07..2358368 100644 > --- a/fs/Kconfig.binfmt > +++ b/fs/Kconfig.binfmt > @@ -36,6 +36,9 @@ config COMPAT_BINFMT_ELF > config ARCH_BINFMT_ELF_STATE > bool > > +config ARCH_HAVE_ELF_PROT > + bool > + > config ARCH_USE_GNU_PROPERTY > bool > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c > index ae345f6..dbfab2e 100644 > --- a/fs/binfmt_elf.c > +++ b/fs/binfmt_elf.c > @@ -531,7 +531,8 @@ static inline int arch_check_elf(struct elfhdr *ehdr, bool has_interp, > > #endif /* !CONFIG_ARCH_BINFMT_ELF_STATE */ > > -static inline int make_prot(u32 p_flags) > +static inline int make_prot(u32 p_flags, struct arch_elf_state *arch_state, > + bool has_interp, bool is_interp) > { > int prot = 0; > > @@ -541,7 +542,8 @@ static inline int make_prot(u32 p_flags) > prot |= PROT_WRITE; > if (p_flags & PF_X) > prot |= PROT_EXEC; > - return prot; > + > + return arch_elf_adjust_prot(prot, arch_state, has_interp, is_interp); > } Random thought: there is already an 'exec-state' structure: bprm. I wonder if arch_state should be attached there instead of passed around here? It'd require almost the same amount of changes, though, so my idea might not gain much in the diffstat, but maybe it's better organizationally? I'm not sure. Otherwise, this all looks fine to me. Reviewed-by: Kees Cook -Kees > > /* This is much more generalized than the library routine read function, > @@ -551,7 +553,8 @@ static inline int make_prot(u32 p_flags) > > static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, > struct file *interpreter, unsigned long *interp_map_addr, > - unsigned long no_base, struct elf_phdr *interp_elf_phdata) > + unsigned long no_base, struct elf_phdr *interp_elf_phdata, > + struct arch_elf_state *arch_state) > { > struct elf_phdr *eppnt; > unsigned long load_addr = 0; > @@ -583,7 +586,8 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, > for (i = 0; i < interp_elf_ex->e_phnum; i++, eppnt++) { > if (eppnt->p_type == PT_LOAD) { > int elf_type = MAP_PRIVATE | MAP_DENYWRITE; > - int elf_prot = make_prot(eppnt->p_flags); > + int elf_prot = make_prot(eppnt->p_flags, arch_state, > + true, true); > unsigned long vaddr = 0; > unsigned long k, map_addr; > > @@ -1040,7 +1044,8 @@ static int load_elf_binary(struct linux_binprm *bprm) > } > } > > - elf_prot = make_prot(elf_ppnt->p_flags); > + elf_prot = make_prot(elf_ppnt->p_flags, &arch_state, > + !!interpreter, false); > > elf_flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE; > > @@ -1186,7 +1191,8 @@ static int load_elf_binary(struct linux_binprm *bprm) > elf_entry = load_elf_interp(&loc->interp_elf_ex, > interpreter, > &interp_map_addr, > - load_bias, interp_elf_phdata); > + load_bias, interp_elf_phdata, > + &arch_state); > if (!IS_ERR((void *)elf_entry)) { > /* > * load_elf_interp() returns relocation > diff --git a/include/linux/elf.h b/include/linux/elf.h > index 7bdc6da..1b6e895 100644 > --- a/include/linux/elf.h > +++ b/include/linux/elf.h > @@ -83,4 +83,16 @@ extern int arch_parse_elf_property(u32 type, const void *data, size_t datasz, > bool compat, struct arch_elf_state *arch); > #endif > > +#ifdef CONFIG_ARCH_HAVE_ELF_PROT > +int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state, > + bool has_interp, bool is_interp); > +#else > +static inline int arch_elf_adjust_prot(int prot, > + const struct arch_elf_state *state, > + bool has_interp, bool is_interp) > +{ > + return prot; > +} > +#endif > + > #endif /* _LINUX_ELF_H */ > -- > 2.1.4 > -- Kees Cook _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel