Received-SPF: pass (google.com: domain of linux-kernel+bounces-56706-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <8fafb8e1-b6be-4d08-945f-b464e3a396c8@I-love.SAKURA.ne.jp>
 <999a4733-c554-43ca-a6e9-998c939fbeb8@I-love.SAKURA.ne.jp> <202402070622.D2DCD9C4@keescook>
In-Reply-To: <202402070622.D2DCD9C4@keescook>
From: Paul Moore <paul@paul-moore.com>
Date: Wed, 7 Feb 2024 10:21:07 -0500
Message-ID: <CAHC9VhTJ85d6jBFBMYUoA4CrYgb6i9yHDC_tFce9ACKi7UTa6Q@mail.gmail.com>
Subject: Re: [PATCH v2 1/3] LSM: add security_execve_abort() hook
To: Kees Cook <keescook@chromium.org>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>, 
	Linus Torvalds <torvalds@linux-foundation.org>, Eric Biederman <ebiederm@xmission.com>, 
	Alexander Viro <viro@zeniv.linux.org.uk>, Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>, 
	James Morris <jmorris@namei.org>, "Serge E. Hallyn" <serge@hallyn.com>, 
	linux-security-module <linux-security-module@vger.kernel.org>, 
	linux-fsdevel <linux-fsdevel@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 7, 2024 at 9:24=E2=80=AFAM Kees Cook <keescook@chromium.org> wr=
ote:
>
> On Sat, Feb 03, 2024 at 07:52:54PM +0900, Tetsuo Handa wrote:
> > A regression caused by commit 978ffcbf00d8 ("execve: open the executabl=
e
> > file before doing anything else") has been fixed by commit 4759ff71f23e
> > ("exec: Check __FMODE_EXEC instead of in_execve for LSMs") and commit
> > 3eab830189d9 ("uselib: remove use of __FMODE_EXEC"). While fixing this
> > regression, Linus commented that we want to remove current->in_execve f=
lag.
> >
> > The current->in_execve flag was introduced by commit f9ce1f1cda8b ("Add
> > in_execve flag into task_struct.") when TOMOYO LSM was merged, and the
> > reason was explained in commit f7433243770c ("LSM adapter functions.").
> >
> > In short, TOMOYO's design is not compatible with COW credential model
> > introduced in Linux 2.6.29, and the current->in_execve flag was added f=
or
> > emulating security_bprm_free() hook which has been removed by introduct=
ion
> > of COW credential model.
> >
> > security_task_alloc()/security_task_free() hooks have been removed by
> > commit f1752eec6145 ("CRED: Detach the credentials from task_struct"),
> > and these hooks have been revived by commit 1a2a4d06e1e9 ("security:
> > create task_free security callback") and commit e4e55b47ed9a ("LSM: Rev=
ive
> > security_task_alloc() hook and per "struct task_struct" security blob."=
).
> >
> > But security_bprm_free() hook did not revive until now. Now that Linus
> > wants TOMOYO to stop carrying state across two independent execve() cal=
ls,
> > and TOMOYO can stop carrying state if a hook for restoring previous sta=
te
> > upon failed execve() call were provided, this patch revives the hook.
> >
> > Since security_bprm_committing_creds() and security_bprm_committed_cred=
s()
> > hooks are called when an execve() request succeeded, we don't need to c=
all
> > security_bprm_free() hook when an execve() request succeeded. Therefore=
,
> > this patch adds security_execve_abort() hook which is called only when =
an
> > execve() request failed after successful prepare_bprm_creds() call.
> >
> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>
> This looks good to me.
>
> Given this touches execve and is related to the recent execve changes,
> shall I carry this in the execve tree for testing and send a PR to Linus
> for it before v6.8 releases?
>
> There's already an Ack from Serge, so this seems a reasonable way to go
> unless Paul would like it done some other way?

Please hold off on this Kees (see my email from yesterday), I'd prefer
to take this via the LSM tree and with the immediate regression
resolved I'd prefer this go in during the upcoming merge window and
not during the -rcX cycle.  Or am I misunderstanding things about the
state of Linus' tree currently?

--
paul-moore.com