Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Thu, 21 Apr 2022 18:24:21 +0100
From:   Catalin Marinas <catalin.marinas@arm.com>
To:     Kees Cook <keescook@chromium.org>
Cc:     Topi Miettinen <toiwoton@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Christoph Hellwig <hch@infradead.org>,
        Lennart Poettering <lennart@poettering.net>,
        Zbigniew =?utf-8?Q?J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>,
        Will Deacon <will@kernel.org>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Eric Biederman <ebiederm@xmission.com>,
        Szabolcs Nagy <szabolcs.nagy@arm.com>,
        Mark Brown <broonie@kernel.org>,
        Jeremy Linton <jeremy.linton@arm.com>, linux-mm@kvack.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-abi-devel@lists.sourceforge.net,
        linux-hardening@vger.kernel.org, Jann Horn <jannh@google.com>,
        Salvatore Mesoraca <s.mesoraca16@gmail.com>,
        Igor Zhbanov <izh1979@gmail.com>
Subject: Re: [PATCH RFC 0/4] mm, arm64: In-kernel support for
 memory-deny-write-execute (MDWE)
Message-ID: <YmGTRQA74n/ZF7Vl@arm.com>
References: <20220413134946.2732468-1-catalin.marinas@arm.com>
 <202204141028.0482B08@keescook>
 <YmAEDsGtxhim46UI@arm.com>
 <c62170c6-5993-2417-4143-5a37a98b227c@gmail.com>
 <202204201610.093C9D5FE8@keescook>
 <YmF5s4KqT5WL4O0G@arm.com>
 <202204210941.4318DE6E8@keescook>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202204210941.4318DE6E8@keescook>
Precedence: bulk

On Thu, Apr 21, 2022 at 09:42:23AM -0700, Kees Cook wrote:
> On Thu, Apr 21, 2022 at 04:35:15PM +0100, Catalin Marinas wrote:
> > Do we want the "was PROT_WRITE" or we just reject mprotect(PROT_EXEC) if
> > the vma is not already PROT_EXEC? The latter is closer to the current
> > systemd approach. The former allows an mprotect(PROT_EXEC) if the
> > mapping was PROT_READ only for example.
> > 
> > I'd drop the "was PROT_WRITE" for now if the aim is a drop-in
> > replacement for BPF MDWE.
> 
> I think "was PROT_WRITE" is an important part of the defense that
> couldn't be done with a simple seccomp filter (which is why the filter
> ended up being a problem in the first place).

I would say "was PROT_WRITE" is slightly more relaxed than "is not
already PROT_EXEC". The seccomp filter can't do "is not already
PROT_EXEC" either since it only checks the mprotect() arguments, not the
current vma flags.

So we have (with sub-cases):

1. Current BPF filter:

   a)	mmap(PROT_READ|PROT_WRITE|PROT_EXEC);	// fails

   b)	mmap(PROT_READ|PROT_EXEC);
	mprotect(PROT_READ|PROT_EXEC|PROT_BTI);	// fails

2. "is not already PROT_EXEC":

   a)	mmap(PROT_READ|PROT_WRITE|PROT_EXEC);	// fails

   b)	mmap(PROT_READ|PROT_EXEC);
	mmap(PROT_READ|PROT_EXEC|PROT_BTI);	// passes

   c)	mmap(PROT_READ);
	mprotect(PROT_READ|PROT_EXEC);		// fails

3. "is or was not PROT_WRITE":

   a)	mmap(PROT_READ|PROT_WRITE|PROT_EXEC);	// fails

   b)	mmap(PROT_READ|PROT_EXEC);
	mmap(PROT_READ|PROT_EXEC|PROT_BTI);	// passes

   c)	mmap(PROT_READ);
	mprotect(PROT_READ|PROT_EXEC);		// passes

   d)	mmap(PROT_READ|PROT_WRITE);
	mprotect(PROT_READ);
	mprotect(PROT_READ|PROT_EXEC);		// fails (was write)

The current seccomp filter is the strictest. If we go for (2), it allows
PROT_BTI as in 2.b but prevents 2.c (as would the current seccomp
filter). (3) relaxes 2.c as in 3.c while still preventing 3.d. Both (1)
and (2) prevent 3.d by simply rejecting mprotect(PROT_EXEC).

If we don't care about 3.c, we might as well go for (2). I don't mind,
already went for (3) in this series. I think either of them would not be
a regression on MDWE, unless there is some test that attempts 3.c and
expects it to fail.

-- 
Catalin