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=1.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,FSL_HELO_FAKE,MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT 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 08FDAC43219 for ; Thu, 25 Apr 2019 20:07:41 +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 0F2C720717 for ; Thu, 25 Apr 2019 20:07:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="t6V7r/p5"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="fvQZt6v5" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0F2C720717 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.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=FmZZDWLk5i2c2L7F75tganI/zfQAQav6e2F8Mt5kqwc=; b=t6V7r/p5YUcyfl eNqsaL/ZgCu2E2RWBhR6/+EQGeyRl5LsowSJagmPw09AK7MqYiA7cGx5gEMU2I5TJH30Um8J9YZtd t4nigg74EKusjq+WDbVvbKLLGQKVU4rvTgj8sIWGp+Xmgq3hyNnLOg5TZJRwXkDg85lvVMus4giVC aaSoiVHxbAlDKo++UXQT1hCewOGRjmxT0R0D23IhljwH+7qLYwOGqmzbhVWV5cu0+ito1WNQI/hr2 btLNqkwE7nNOhWW1j5SzDZ1ia7757+0qZTgAqa6V+/IHc2qinjdI/tyEpndElNHnxtZotG+W1gcys q7n1HTZJsowXguWYMYcg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hJkec-0003L0-1i; Thu, 25 Apr 2019 20:07:34 +0000 Received: from mail-wm1-x341.google.com ([2a00:1450:4864:20::341]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hJkeY-0003Ke-Jq for linux-arm-kernel@lists.infradead.org; Thu, 25 Apr 2019 20:07:32 +0000 Received: by mail-wm1-x341.google.com with SMTP id v14so984776wmf.2 for ; Thu, 25 Apr 2019 13:07:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=EW2AmtlR46Ljm42I0M/vtZb/UWMcTt9/zEqoHK9o+e8=; b=fvQZt6v5pao05bSag5VAtZrLsazVQtfyBZktYzW/f3nEL4ZZ6vg6XY2jMsUm1CC0Rv WzkyWN4+33Fry35ne8+Y8ql7ShstixbfULTqre1T+yYfBTonDvP3AGhqWR1LtGsLcIeo JXznUyYuvMba2e2pY04yoWM9RSWabGngiv5OBPoMQLl731L7FG1HALEsIb744Yt/Kqtj t/5+E4Gd67zVTDFZkDHgjPXmGKjVAiI9kNIygYe52QRJ6JiX78mszCCmxY7QRXu/2HBn XyppScdvcPW/zHv1ZUiqOf21DWWNTM+1z9CfVn7x43ftzpBWPMdHbhFmWL6kZabefOoC HMrg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=EW2AmtlR46Ljm42I0M/vtZb/UWMcTt9/zEqoHK9o+e8=; b=F+pBpIjvfktTlO04ByImZ0ub4F5+f7qf6LIVEVzeeVQTi04jq4QJludE6jK3hiSEtf OTidgp3AuzdxYdkfioDXWRWkqiFpFR2PnT+IFyBfejSiESWMoZkWozhgmjbv6WZgDLsk XcyLUBWDywAYHU1cGfVBl5eETofOKk0BAsS0ffUBVa4AzDtegqNQTrAZmrPETEw1mX/o o+0BQ3yZN255QpHfouNkjNSZmowRsJpa5WO/yJysrWOHxn/dqiLC0yH/DWerVeHxI8kW 2U6SgEYQ1gC6QVwr/ZI0lQJTVSKaQz24gzYz+e/v0iPKmVzfcVrGTB041z5QV5TutWEE XVmw== X-Gm-Message-State: APjAAAXjx8d49TWowZhuyzsxtRy02bXOGUsQHa3aNF50tPpH5PkKE5yC hNTfiJPETgrQ7H1AX0w3COA= X-Google-Smtp-Source: APXvYqw9uxtLpKWI/JE4lIio4jgBTyJGRKdzpwAjslqwO75l2ZDoEprXoiFhg5pCeStmZLh6mpNl+A== X-Received: by 2002:a1c:9dc8:: with SMTP id g191mr4750266wme.132.1556222848897; Thu, 25 Apr 2019 13:07:28 -0700 (PDT) Received: from gmail.com (2E8B0CD5.catv.pool.telekom.hu. [46.139.12.213]) by smtp.gmail.com with ESMTPSA id s7sm6892393wrn.84.2019.04.25.13.07.27 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 25 Apr 2019 13:07:28 -0700 (PDT) Date: Thu, 25 Apr 2019 22:07:25 +0200 From: Ingo Molnar To: Kees Cook Subject: Re: [PATCH v2] binfmt_elf: Update READ_IMPLIES_EXEC logic for modern CPUs Message-ID: <20190425200725.GC58719@gmail.com> References: <20190424203408.GA11386@beast> <20190425054242.GA7816@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190425_130730_686729_D5D913A8 X-CRM114-Status: GOOD ( 34.09 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Stephen Rothwell , Peter Zijlstra , Arnd Bergmann , Marc Gonzalez , Hector Marco-Gisbert , X86 ML , Will Deacon , LKML , Andy Lutomirski , Jason Gunthorpe , Borislav Petkov , Catalin Marinas , Kernel Hardening , Andrew Morton , Linus Torvalds , Thomas Gleixner , Linux ARM 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 * Kees Cook wrote: > On Wed, Apr 24, 2019 at 10:42 PM Ingo Molnar wrote: > > Just to make clear, is the change from the old behavior, in essence: > > > > > > CPU: | lacks NX | has NX, ia32 | has NX, x86_64 | > > ELF: | | | | > > ------------------------------|------------------|------------------| > > missing GNU_STACK | exec-all | exec-all | exec-none | > > - GNU_STACK == RWX | exec-all | exec-all | exec-all | > > + GNU_STACK == RWX | exec-all | exec-stack | exec-stack | > > GNU_STACK == RW | exec-all | exec-none | exec-none | > > [...] > > 'exec-all' : all user mappings are executable > > For extreme clarity, this should be: > > 'exec-all' : all PROT_READ user mappings are executable, except when > backed by files on a noexec-filesystem. > > > 'exec-none' : only PROT_EXEC user mappings are executable > > 'exec-stack': only the stack and PROT_EXEC user mappings are executable > > Thanks for helping clarify this. I spent last evening trying to figure > out a better way to explain/illustrate this series; my prior patch > combines too many things into a single change. One thing I noticed is > the "lacks NX" column is wrong: for "lack NX", our current state is > "don't care". If we _add_ RIE for the "lacks NX" case unconditionally, > we may cause unexpected problems[1]. More on this below... So what does RIE in the !NX case do to regular RAM (with the exception of device memory, see below), does it actively reject or modify actual mmap() calls and introduces behavioral changes, or is it mostly just the /proc reporting of permission bits? If it's just reporting, with no (intended) behavioral side effects, then is there really a true difference? > But yes, your above diff for "has NX" is roughly correct. I'll walk > through each piece I'm thinking about. Here is the current state: > > CPU: | lacks NX* | has NX, ia32 | has NX, x86_64 | > ELF: | | | | > -------------------------------|------------------|----------------| > missing GNU_STACK | exec-all | exec-all | exec-all | > GNU_STACK == RWX | exec-all | exec-all | exec-all | > GNU_STACK == RW | exec-none | exec-none | exec-none | > > *this column has no architecture effect: NX markings are ignored by > hardware, but may have behavioral effects when "wants X" collides with > "cannot be X" constraints in memory permission flags, as in [1]. So [1] appears to be device driver mapping a BAR that isn't intended to be excutable: https://lore.kernel.org/netdev/20190418055759.GA3155@mellanox.com/ and the question is, do we reject this at the device driver mmap() level already, right? I suspect the best behavior is to reject as early as possible, so I agree with your change here - even though !NX systems tend to become less and less relevant these days. ( User-space can still work it around in practice by not using PROT_EXEC and sending CPU execution there - with undefined/undesirable outcomes, but that's user-space getting what they are asking for. ) > I want to make three changes, listed in increasing risk levels. > > First, I want to split "missing GNU_STACK" and "GNU_STACK == RWX", > which is currently causing expected behavior for driver mmap > regions[1], etc: > > CPU: | lacks NX* | has NX, ia32 | has NX, x86_64 | > ELF: | | | | > -------------------------------|------------------|----------------| > missing GNU_STACK | exec-all | exec-all | exec-all | > - GNU_STACK == RWX | exec-all | exec-all | exec-all | > + GNU_STACK == RWX | exec-stack | exec-stack | exec-stack | > GNU_STACK == RW | exec-none | exec-none | exec-none | > > AFAICT, this has the least risk. I'm not aware of any situation where > GNU_STACK==RWX is supposed to mean MORE than that. As Jann researched, > even thread stacks will be treated correctly[2]. The risk would be > discovering some use-case where a program was executing memory that it > had not explicitly marked as executable. For ELFs marked with > GNU_STACK, this seems unlikely (I hope). Ack: and this actively increases security for GNU_STACK=RWX executables, as it modifies exec-all to exec-stack, which narrows executability in a real way, and enforced by NX CPUs both in 64-bit and 32-bit apps. While obviously the executable stack is a gaping hole in the typical case, not all attacks can utilize an executable stack and they might be able to utilize other W+X regions such as the heap or some data mmap() area, right? BTW., do we have any compat variations with the table, i.e. tasks running on a 32-bit kernel versus running in 32-bit mode on a 64-bit kernel? I.e. should there be another column for compat, or is compat behavior always the same as 32-bit kernel behavior? > Second, I want to split the behavior of "missing GNU_STACK" between > ia32 and x86_64. The reasonable(?) default for x86_64 memory is for it > to be NX. For the very rare x86_64 systems that do not have NX, this > shouldn't change anything because they still fall into the "don't > care" column. It would look like this: > > CPU: | lacks NX* | has NX, ia32 | has NX, x86_64 | > ELF: | | | | > -------------------------------|------------------|----------------| > - missing GNU_STACK | exec-all | exec-all | exec-all | > + missing GNU_STACK | exec-all | exec-all | exec-none | > GNU_STACK == RWX | exec-stack | exec-stack | exec-stack | > GNU_STACK == RW | exec-none | exec-none | exec-none | > > This carries some risk that there are ancient x86_64 binaries that > still behave like their even more ancient ia32 counterparts, and > expect to be able to execute any memory. I would _hope_ this is rare, > but I have no way to actually know if things like this exist in the > real world. Ack: this too actively restricts executability which is the right direction to go. (Absent reported regressions.) > Third, I want to have the "lacks NX" column actually reflect reality. > Right now on such a system, memory permissions will show "not > executable" but there is actually no architectural checking for these > permissions. I think the true nature of such a system should be > reflected in the reported permissions. It would look like this: > > CPU: | lacks NX* | has NX, ia32 | has NX, x86_64 | > ELF: | | | | > -------------------------------|------------------|----------------| > missing GNU_STACK | exec-all | exec-all | exec-none | > - GNU_STACK == RWX | exec-stack | exec-stack | exec-stack | > - GNU_STACK == RW | exec-none | exec-none | exec-none | > + GNU_STACK == RWX | exec-all | exec-stack | exec-stack | > + GNU_STACK == RW | exec-all | exec-none | exec-none | > > This carries the largest risk because it effectively enables > READ_IMPLIES_EXEC on all processes for such systems. I worry this > might trip as-yet-unseen problems like in [1], for only cosmetic > improvements. > > My intention was to split up the series and likely not even bother > with the third change, since it feels like too high a risk to me. What > do you think? So what's the benefit of this third phase, more transparency because reported permissions and API behavior will match reality? Can we do something else perhaps and do phase 3 change for *RAM* backed vmas, but be more restrictive for device mappings, allowing [1] to be handled better? I suspect there will be a complexity threshold where it's better to default to the simpler approach though, especially since !NX gets so little attention and testing these days. So I'd be fine with phase 3 too. I'd definitely suggest making this 3 separate patches, so any regressions can be tracked back to the specific change that triggers it. Thanks, Ingo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel