Re: [PATCH v6 4/7] fs: Introduce O_MAYEXEC flag for openat2(2)

From: Kees Cook <keescook@chromium.org>
To: "Mickaël Salaün" <mic@digikod.net>
Cc: "Jan Kara" <jack@suse.cz>,
	"Matthew Bobrowski" <mbobrowski@mbobrowski.org>,
	linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Aleksa Sarai" <cyphar@cyphar.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Christian Brauner" <christian.brauner@ubuntu.com>,
	"Christian Heimes" <christian@python.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Deven Bowers" <deven.desai@linux.microsoft.com>,
	"Dmitry Vyukov" <dvyukov@google.com>,
	"Eric Biggers" <ebiggers@kernel.org>,
	"Eric Chiang" <ericchiang@google.com>,
	"Florian Weimer" <fweimer@redhat.com>,
	"James Morris" <jmorris@namei.org>,
	"Jann Horn" <jannh@google.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Lakshmi Ramasubramanian" <nramas@linux.microsoft.com>,
	"Matthew Garrett" <mjg59@google.com>,
	"Matthew Wilcox" <willy@infradead.org>,
	"Michael Kerrisk" <mtk.manpages@gmail.com>,
	"Mickaël Salaün" <mickael.salaun@ssi.gouv.fr>,
	"Mimi Zohar" <zohar@linux.ibm.com>,
	"Philippe Trébuchet" <philippe.trebuchet@ssi.gouv.fr>,
	"Scott Shell" <scottsh@microsoft.com>,
	"Sean Christopherson" <sean.j.christopherson@intel.com>,
	"Shuah Khan" <shuah@kernel.org>,
	"Steve Dower" <steve.dower@python.org>,
	"Steve Grubb" <sgrubb@redhat.com>,
	"Tetsuo Handa" <penguin-kernel@I-love.SAKURA.ne.jp>,
	"Thibaut Sautereau" <thibaut.sautereau@ssi.gouv.fr>,
	"Vincent Strubel" <vincent.strubel@ssi.gouv.fr>,
	kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v6 4/7] fs: Introduce O_MAYEXEC flag for openat2(2)
Date: Thu, 16 Jul 2020 08:31:32 -0700	[thread overview]
Message-ID: <202007160822.CCDB5478@keescook> (raw)
In-Reply-To: <b209ea10-5b7f-c40e-5b6a-3da9028403d5@digikod.net>

On Thu, Jul 16, 2020 at 04:18:27PM +0200, Mickaël Salaün wrote:
> On 15/07/2020 22:06, Kees Cook wrote:
> > On Tue, Jul 14, 2020 at 08:16:35PM +0200, Mickaël Salaün wrote:
> >> The implementation of O_MAYEXEC almost duplicates what execve(2) and
> >> uselib(2) are already doing: setting MAY_OPENEXEC in acc_mode (which can
> >> then be checked as MAY_EXEC, if enforced), and propagating FMODE_EXEC to
> >> _fmode via __FMODE_EXEC flag (which can then trigger a
> >> fanotify/FAN_OPEN_EXEC event).
> >> [...]
> > 
> > Adding __FMODE_EXEC here will immediately change the behaviors of NFS
> > and fsnotify. If that's going to happen, I think it needs to be under
> > the control of the later patches doing the behavioral controls.
> > (specifically, NFS looks like it completely changes its access control
> > test when this is set and ignores the read/write checks entirely, which
> > is not what's wanted).
> 
> __FMODE_EXEC was suggested by Jan Kara and Matthew Bobrowski because of
> fsnotify. However, the NFS handling of SUID binaries [1] indeed leads to
> an unintended behavior. This also means that uselib(2) shouldn't work
> properly with NFS. I can remove the __FMODE_EXEC flag for now.

I kind of wonder if we need to more completely fix __FMODE_EXEC?

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=f8d9a897d4384b77f13781ea813156568f68b83e

Hmpf, this implies that "fmode" should contain MAY_EXEC? It really looks
like __FMODE_EXEC is a hack for places where only "flags" were passed
around, and this only seems to be an issue for NFS at this point? And it
should be fixable for fsnotify too?

Hmm. (And nothing should use uselib anyway...)

-- 
Kees Cook