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=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 041BECA9EA0 for ; Fri, 18 Oct 2019 11:16:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D4050222BD for ; Fri, 18 Oct 2019 11:16:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2442549AbfJRLQc (ORCPT ); Fri, 18 Oct 2019 07:16:32 -0400 Received: from [217.140.110.172] ([217.140.110.172]:35572 "EHLO foss.arm.com" rhost-flags-FAIL-FAIL-OK-OK) by vger.kernel.org with ESMTP id S2404572AbfJRLQc (ORCPT ); Fri, 18 Oct 2019 07:16:32 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 18428B56; Fri, 18 Oct 2019 04:16:09 -0700 (PDT) Received: from lakrids.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D4D4D3F6C4; Fri, 18 Oct 2019 04:16:05 -0700 (PDT) Date: Fri, 18 Oct 2019 12:16:03 +0100 From: Mark Rutland To: Dave Martin Cc: Paul Elliott , Peter Zijlstra , Catalin Marinas , Will Deacon , Yu-cheng Yu , Amit Kachhap , Vincenzo Frascino , linux-arch@vger.kernel.org, Eugene Syromiatnikov , Szabolcs Nagy , "H.J. Lu" , Andrew Jones , Kees Cook , 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 , Dave Kleikamp , Benjamin Herrenschmidt , Andrew Morton Subject: Re: [PATCH v2 05/12] arm64: Basic Branch Target Identification support Message-ID: <20191018111603.GD27759@lakrids.cambridge.arm.com> References: <1570733080-21015-1-git-send-email-Dave.Martin@arm.com> <1570733080-21015-6-git-send-email-Dave.Martin@arm.com> <20191011151028.GE33537@lakrids.cambridge.arm.com> <20191011172013.GQ27757@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191011172013.GQ27757@arm.com> User-Agent: Mutt/1.11.1+11 (2f07cb52) (2018-12-01) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org [adding mm folk] On Fri, Oct 11, 2019 at 06:20:15PM +0100, Dave Martin wrote: > On Fri, Oct 11, 2019 at 04:10:29PM +0100, Mark Rutland wrote: > > On Thu, Oct 10, 2019 at 07:44:33PM +0100, Dave Martin wrote: > > > +#define arch_validate_prot(prot, addr) arm64_validate_prot(prot, addr) > > > +static inline int arm64_validate_prot(unsigned long prot, unsigned long addr) > > > +{ > > > + unsigned long supported = PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM; > > > + > > > + if (system_supports_bti()) > > > + supported |= PROT_BTI; > > > + > > > + return (prot & ~supported) == 0; > > > +} > > > > If we have this check, can we ever get into arm64_calc_vm_prot_bits() > > with PROT_BIT but !system_supports_bti()? > > > > ... or can that become: > > > > return (prot & PROT_BTI) ? VM_ARM64_BTI : 0; > > We can reach this via mmap() and friends IIUC. > > Since this function only gets called once-ish per vma I have a weak > preference for keeping the check here to avoid code fragility. > > > It does feel like arch_validate_prot() is supposed to be a generic gate > for prot flags coming into the kernel via any route though, but only the > mprotect() path actually uses it. > > This function originally landed in v2.6.27 as part of the powerpc strong > access ordering support (PROT_SAO): > > b845f313d78e ("mm: Allow architectures to define additional protection bits") > ef3d3246a0d0 ("powerpc/mm: Add Strong Access Ordering support") > > where the mmap() path uses arch_calc_vm_prot_bits() without > arch_validate_prot(), just as in the current code. powerpc's original > arch_calc_vm_prot_bits() does no obvious policing. > > This might be a bug. I can draft a patch to add it for the mmap() path > for people to comment on ... I can't figure out yet whether or not the > difference is intentional or there's some subtlety that I'm missed. >From reading those two commit messages, it looks like this was an oversight. I'd expect that we should apply this check for any user-provided prot (i.e. it should apply to both mprotect and mmap). Ben, Andrew, does that make sense to you? ... or was there some reason to only do this for mprotect? Thanks, Mark. > mmap( ... prot = -1 ... ) succeeds with effective rwx permissions and no > apparent ill effects on my random x86 box, but mprotect(..., -1) fails > with -EINVAL. > > This is at least strange. > > Theoretically, tightening this would be an ABI break, though I'd say > this behaviour is not intentional. > > Thoughts? > > [...] > > Cheers > ---Dave From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Rutland Subject: Re: [PATCH v2 05/12] arm64: Basic Branch Target Identification support Date: Fri, 18 Oct 2019 12:16:03 +0100 Message-ID: <20191018111603.GD27759@lakrids.cambridge.arm.com> References: <1570733080-21015-1-git-send-email-Dave.Martin@arm.com> <1570733080-21015-6-git-send-email-Dave.Martin@arm.com> <20191011151028.GE33537@lakrids.cambridge.arm.com> <20191011172013.GQ27757@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20191011172013.GQ27757@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Dave Martin Cc: Dave Kleikamp , Paul Elliott , Peter Zijlstra , Catalin Marinas , Will Deacon , Andrew Jones , Amit Kachhap , Vincenzo Frascino , linux-arch@vger.kernel.org, Eugene Syromiatnikov , Szabolcs Nagy , Benjamin Herrenschmidt , "H.J. Lu" , Yu-cheng Yu , Kees Cook , 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 List-Id: linux-arch.vger.kernel.org [adding mm folk] On Fri, Oct 11, 2019 at 06:20:15PM +0100, Dave Martin wrote: > On Fri, Oct 11, 2019 at 04:10:29PM +0100, Mark Rutland wrote: > > On Thu, Oct 10, 2019 at 07:44:33PM +0100, Dave Martin wrote: > > > +#define arch_validate_prot(prot, addr) arm64_validate_prot(prot, addr) > > > +static inline int arm64_validate_prot(unsigned long prot, unsigned long addr) > > > +{ > > > + unsigned long supported = PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM; > > > + > > > + if (system_supports_bti()) > > > + supported |= PROT_BTI; > > > + > > > + return (prot & ~supported) == 0; > > > +} > > > > If we have this check, can we ever get into arm64_calc_vm_prot_bits() > > with PROT_BIT but !system_supports_bti()? > > > > ... or can that become: > > > > return (prot & PROT_BTI) ? VM_ARM64_BTI : 0; > > We can reach this via mmap() and friends IIUC. > > Since this function only gets called once-ish per vma I have a weak > preference for keeping the check here to avoid code fragility. > > > It does feel like arch_validate_prot() is supposed to be a generic gate > for prot flags coming into the kernel via any route though, but only the > mprotect() path actually uses it. > > This function originally landed in v2.6.27 as part of the powerpc strong > access ordering support (PROT_SAO): > > b845f313d78e ("mm: Allow architectures to define additional protection bits") > ef3d3246a0d0 ("powerpc/mm: Add Strong Access Ordering support") > > where the mmap() path uses arch_calc_vm_prot_bits() without > arch_validate_prot(), just as in the current code. powerpc's original > arch_calc_vm_prot_bits() does no obvious policing. > > This might be a bug. I can draft a patch to add it for the mmap() path > for people to comment on ... I can't figure out yet whether or not the > difference is intentional or there's some subtlety that I'm missed. >From reading those two commit messages, it looks like this was an oversight. I'd expect that we should apply this check for any user-provided prot (i.e. it should apply to both mprotect and mmap). Ben, Andrew, does that make sense to you? ... or was there some reason to only do this for mprotect? Thanks, Mark. > mmap( ... prot = -1 ... ) succeeds with effective rwx permissions and no > apparent ill effects on my random x86 box, but mprotect(..., -1) fails > with -EINVAL. > > This is at least strange. > > Theoretically, tightening this would be an ABI break, though I'd say > this behaviour is not intentional. > > Thoughts? > > [...] > > Cheers > ---Dave From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [217.140.110.172] ([217.140.110.172]:35572 "EHLO foss.arm.com" rhost-flags-FAIL-FAIL-OK-OK) by vger.kernel.org with ESMTP id S2404572AbfJRLQc (ORCPT ); Fri, 18 Oct 2019 07:16:32 -0400 Date: Fri, 18 Oct 2019 12:16:03 +0100 From: Mark Rutland Subject: Re: [PATCH v2 05/12] arm64: Basic Branch Target Identification support Message-ID: <20191018111603.GD27759@lakrids.cambridge.arm.com> References: <1570733080-21015-1-git-send-email-Dave.Martin@arm.com> <1570733080-21015-6-git-send-email-Dave.Martin@arm.com> <20191011151028.GE33537@lakrids.cambridge.arm.com> <20191011172013.GQ27757@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191011172013.GQ27757@arm.com> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Dave Martin Cc: Paul Elliott , Peter Zijlstra , Catalin Marinas , Will Deacon , Yu-cheng Yu , Amit Kachhap , Vincenzo Frascino , linux-arch@vger.kernel.org, Eugene Syromiatnikov , Szabolcs Nagy , "H.J. Lu" , Andrew Jones , Kees Cook , 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 , Dave Kleikamp , Benjamin Herrenschmidt , Andrew Morton Message-ID: <20191018111603.JkvgkkvhIib1-oRlLc1QBbqXlzjWMA_-jWfpCuxIVFo@z> [adding mm folk] On Fri, Oct 11, 2019 at 06:20:15PM +0100, Dave Martin wrote: > On Fri, Oct 11, 2019 at 04:10:29PM +0100, Mark Rutland wrote: > > On Thu, Oct 10, 2019 at 07:44:33PM +0100, Dave Martin wrote: > > > +#define arch_validate_prot(prot, addr) arm64_validate_prot(prot, addr) > > > +static inline int arm64_validate_prot(unsigned long prot, unsigned long addr) > > > +{ > > > + unsigned long supported = PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM; > > > + > > > + if (system_supports_bti()) > > > + supported |= PROT_BTI; > > > + > > > + return (prot & ~supported) == 0; > > > +} > > > > If we have this check, can we ever get into arm64_calc_vm_prot_bits() > > with PROT_BIT but !system_supports_bti()? > > > > ... or can that become: > > > > return (prot & PROT_BTI) ? VM_ARM64_BTI : 0; > > We can reach this via mmap() and friends IIUC. > > Since this function only gets called once-ish per vma I have a weak > preference for keeping the check here to avoid code fragility. > > > It does feel like arch_validate_prot() is supposed to be a generic gate > for prot flags coming into the kernel via any route though, but only the > mprotect() path actually uses it. > > This function originally landed in v2.6.27 as part of the powerpc strong > access ordering support (PROT_SAO): > > b845f313d78e ("mm: Allow architectures to define additional protection bits") > ef3d3246a0d0 ("powerpc/mm: Add Strong Access Ordering support") > > where the mmap() path uses arch_calc_vm_prot_bits() without > arch_validate_prot(), just as in the current code. powerpc's original > arch_calc_vm_prot_bits() does no obvious policing. > > This might be a bug. I can draft a patch to add it for the mmap() path > for people to comment on ... I can't figure out yet whether or not the > difference is intentional or there's some subtlety that I'm missed. 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=-2.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 21E71CA9EA0 for ; Fri, 18 Oct 2019 11:16:29 +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 D91CF20663 for ; Fri, 18 Oct 2019 11:16:28 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ORBCcRg1" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D91CF20663 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com 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=1dYWVEDNryokK1J0AAi3PC9K7VYWqMlMj/ydfiIcZxk=; b=ORBCcRg1Xv2dZP OeJ9v7vMHNKCxrWqabme6ve//XDXEoAFmfpdAJ0nSAXHRdjaaPmts9xZmv46dNpI4aW4qc4A1JOFB Bezmm8C0ez4ZF2LZwNGRQ0Kr0XMI+MDGOf3yKYJoJltbUnjJIJKVLPS0Oi4UceX0tnEjG1myPkd9r QU/32v1CcX1+NnFGDE2x98XFB4qwC/VQO/DkK/B8tg4tgFfIIzaiAEJuuKHp44bQJTcOYQ3Nqg9Nj eiQW/QcTPFJ8DINg0OcVklGFMS7+uepGDZy+3ooEo+IdZnqm7fwf8M0wFKXtWkydG4La5XM7GOeMT F5EWT8QK0sWqcm+fJwlA==; 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 1iLQF5-0000AU-A8; Fri, 18 Oct 2019 11:16:23 +0000 Received: from [217.140.110.172] (helo=foss.arm.com) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iLQF1-00009Q-VJ for linux-arm-kernel@lists.infradead.org; Fri, 18 Oct 2019 11:16:21 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 18428B56; Fri, 18 Oct 2019 04:16:09 -0700 (PDT) Received: from lakrids.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D4D4D3F6C4; Fri, 18 Oct 2019 04:16:05 -0700 (PDT) Date: Fri, 18 Oct 2019 12:16:03 +0100 From: Mark Rutland To: Dave Martin Subject: Re: [PATCH v2 05/12] arm64: Basic Branch Target Identification support Message-ID: <20191018111603.GD27759@lakrids.cambridge.arm.com> References: <1570733080-21015-1-git-send-email-Dave.Martin@arm.com> <1570733080-21015-6-git-send-email-Dave.Martin@arm.com> <20191011151028.GE33537@lakrids.cambridge.arm.com> <20191011172013.GQ27757@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20191011172013.GQ27757@arm.com> User-Agent: Mutt/1.11.1+11 (2f07cb52) (2018-12-01) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191018_041620_101126_6200041B X-CRM114-Status: GOOD ( 24.60 ) 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: Dave Kleikamp , Paul Elliott , Peter Zijlstra , Catalin Marinas , Will Deacon , Andrew Jones , Amit Kachhap , Vincenzo Frascino , linux-arch@vger.kernel.org, Eugene Syromiatnikov , Szabolcs Nagy , Benjamin Herrenschmidt , "H.J. Lu" , Yu-cheng Yu , Kees Cook , 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, Andrew Morton , 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 [adding mm folk] On Fri, Oct 11, 2019 at 06:20:15PM +0100, Dave Martin wrote: > On Fri, Oct 11, 2019 at 04:10:29PM +0100, Mark Rutland wrote: > > On Thu, Oct 10, 2019 at 07:44:33PM +0100, Dave Martin wrote: > > > +#define arch_validate_prot(prot, addr) arm64_validate_prot(prot, addr) > > > +static inline int arm64_validate_prot(unsigned long prot, unsigned long addr) > > > +{ > > > + unsigned long supported = PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM; > > > + > > > + if (system_supports_bti()) > > > + supported |= PROT_BTI; > > > + > > > + return (prot & ~supported) == 0; > > > +} > > > > If we have this check, can we ever get into arm64_calc_vm_prot_bits() > > with PROT_BIT but !system_supports_bti()? > > > > ... or can that become: > > > > return (prot & PROT_BTI) ? VM_ARM64_BTI : 0; > > We can reach this via mmap() and friends IIUC. > > Since this function only gets called once-ish per vma I have a weak > preference for keeping the check here to avoid code fragility. > > > It does feel like arch_validate_prot() is supposed to be a generic gate > for prot flags coming into the kernel via any route though, but only the > mprotect() path actually uses it. > > This function originally landed in v2.6.27 as part of the powerpc strong > access ordering support (PROT_SAO): > > b845f313d78e ("mm: Allow architectures to define additional protection bits") > ef3d3246a0d0 ("powerpc/mm: Add Strong Access Ordering support") > > where the mmap() path uses arch_calc_vm_prot_bits() without > arch_validate_prot(), just as in the current code. powerpc's original > arch_calc_vm_prot_bits() does no obvious policing. > > This might be a bug. I can draft a patch to add it for the mmap() path > for people to comment on ... I can't figure out yet whether or not the > difference is intentional or there's some subtlety that I'm missed. >From reading those two commit messages, it looks like this was an oversight. I'd expect that we should apply this check for any user-provided prot (i.e. it should apply to both mprotect and mmap). Ben, Andrew, does that make sense to you? ... or was there some reason to only do this for mprotect? Thanks, Mark. > mmap( ... prot = -1 ... ) succeeds with effective rwx permissions and no > apparent ill effects on my random x86 box, but mprotect(..., -1) fails > with -EINVAL. > > This is at least strange. > > Theoretically, tightening this would be an ABI break, though I'd say > this behaviour is not intentional. > > Thoughts? > > [...] > > Cheers > ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel