Linux-Security-Module Archive mirror
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Paul Moore <paul@paul-moore.com>,
	Eric Biederman <ebiederm@xmission.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-security-module <linux-security-module@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Serge Hallyn <serge@hallyn.com>
Subject: Re: [PATCH v3 1/3] LSM: add security_execve_abort() hook
Date: Wed, 1 May 2024 13:04:56 -0700	[thread overview]
Message-ID: <202405011257.E590171@keescook> (raw)
In-Reply-To: <76bcd199-6c14-484f-8d4d-5a9c4a07ff7b@I-love.SAKURA.ne.jp>

On Thu, Feb 15, 2024 at 11:33:32PM +0900, Tetsuo Handa wrote:
> On 2024/02/15 6:46, Paul Moore wrote:
> >> To quickly summarize, there are two paths forward that I believe are
> >> acceptable from a LSM perspective, pick either one and send me an
> >> updated patchset.
> >>
> >> 1. Rename the hook to security_bprm_free() and update the LSM hook
> >> description as I mentioned earlier in this thread.
> >>
> >> 2. Rename the hook to security_execve_revert(), move it into the
> >> execve related functions, and update the LSM hook description to
> >> reflect that this hook is for reverting execve related changes to the
> >> current task's internal LSM state beyond what is possible via the
> >> credential hooks.
> > 
> > Hi Tetsuo, I just wanted to check on this and see if you've been able
> > to make any progress?
> > 
> 
> I'm fine with either approach. Just worrying that someone doesn't like
> overhead of unconditionally calling security_bprm_free() hook.

With the coming static calls series, this concern will delightfully go
away. :)

> If everyone is fine with below one, I'll post v4 patchset.

I'm okay with it being security_bprm_free(). One question I had was how
Tomoyo deals with it? I was depending on the earlier hook only being
called in a failure path.

> [...]
> @@ -1530,6 +1530,7 @@ static void free_bprm(struct linux_binprm *bprm)
>  		kfree(bprm->interp);
>  	kfree(bprm->fdpath);
>  	kfree(bprm);
> +	security_bprm_free();
>  }

I'm fine with security_bprm_free(), but this needs to be moved to the
start of free_bprm(), and to pass the bprm itself. This is the pattern we
use for all the other "free" hooks. (Though in this case we don't attach
any security context to the brpm, but there may be state of interest in
it.) i.e.:

diff --git a/fs/exec.c b/fs/exec.c
index 40073142288f..7ec13b104960 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1532,6 +1532,7 @@ static void do_close_execat(struct file *file)
 
 static void free_bprm(struct linux_binprm *bprm)
 {
+	security_bprm_free(bprm);
 	if (bprm->mm) {
 		acct_arg_size(bprm, 0);
 		mmput(bprm->mm);

-- 
Kees Cook

  parent reply	other threads:[~2024-05-01 20:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-06 13:58 [PATCH v3 0/3] fs/exec: remove current->in_execve flag Tetsuo Handa
2024-02-06 13:59 ` [PATCH v3 1/3] LSM: add security_execve_abort() hook Tetsuo Handa
2024-02-07  0:00   ` Paul Moore
2024-02-07 11:10     ` Tetsuo Handa
2024-02-07 14:34       ` Kees Cook
2024-02-07 16:01         ` Paul Moore
2024-02-14 21:46           ` Paul Moore
2024-02-15 14:33             ` Tetsuo Handa
2024-02-15 23:47               ` Paul Moore
2024-05-01 20:04               ` Kees Cook [this message]
2024-02-06 13:59 ` [PATCH v3 2/3] tomoyo: replace current->in_execve flag with " Tetsuo Handa
2024-02-06 13:59 ` [PATCH v3 3/3] fs/exec: remove current->in_execve flag Tetsuo Handa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202405011257.E590171@keescook \
    --to=keescook@chromium.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=serge@hallyn.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).