[PATCH v2 0/3] fs/exec: remove current->in_execve flag

[PATCH v2 0/3] fs/exec: remove current->in_execve flag

[Tetsuo Handa]

This is a follow up series for removing current->in_execve flag.

https://lkml.kernel.org/r/b5a12ecd-468d-4b50-9f8c-17ae2a2560b4@I-love.SAKURA.ne.jp

[PATCH v2 1/3] LSM: add security_execve_abort() hook
[PATCH v2 2/3] tomoyo: replace current->in_execve flag with security_execve_abort() hook
[PATCH v2 3/3] fs/exec: remove current->in_execve flag

 fs/exec.c                     |    4 +---
 include/linux/lsm_hook_defs.h |    1 +
 include/linux/sched.h         |    3 ---
 include/linux/security.h      |    5 +++++
 security/security.c           |   11 +++++++++++
 security/tomoyo/tomoyo.c      |   22 ++++++----------------
 6 files changed, 24 insertions(+), 22 deletions(-)

Changes in v2:

  Replace security_bprm_aborting_creds(const struct linux_binprm *bprm) with
  security_execve_abort(void), suggested by Eric W. Biederman.

[PATCH v2 1/3] LSM: add security_execve_abort() hook

[Tetsuo Handa]

A regression caused by commit 978ffcbf00d8 ("execve: open the executable
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 flag.

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 for
emulating security_bprm_free() hook which has been removed by introduction
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: Revive
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() calls,
and TOMOYO can stop carrying state if a hook for restoring previous state
upon failed execve() call were provided, this patch revives the hook.

Since security_bprm_committing_creds() and security_bprm_committed_creds()
hooks are called when an execve() request succeeded, we don't need to call
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>
---
 fs/exec.c                     |  1 +
 include/linux/lsm_hook_defs.h |  1 +
 include/linux/security.h      |  5 +++++
 security/security.c           | 11 +++++++++++
 4 files changed, 18 insertions(+)

diff --git a/fs/exec.c b/fs/exec.c
index af4fbb61cd53..d6d35a06fd08 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1521,6 +1521,7 @@ static void free_bprm(struct linux_binprm *bprm)
 	if (bprm->cred) {
 		mutex_unlock(&current->signal->cred_guard_mutex);
 		abort_creds(bprm->cred);
+		security_execve_abort();
 	}
 	do_close_execat(bprm->file);
 	if (bprm->executable)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 76458b6d53da..fd100ab71a33 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -54,6 +54,7 @@ LSM_HOOK(int, 0, bprm_creds_from_file, struct linux_binprm *bprm, const struct f
 LSM_HOOK(int, 0, bprm_check_security, struct linux_binprm *bprm)
 LSM_HOOK(void, LSM_RET_VOID, bprm_committing_creds, const struct linux_binprm *bprm)
 LSM_HOOK(void, LSM_RET_VOID, bprm_committed_creds, const struct linux_binprm *bprm)
+LSM_HOOK(void, LSM_RET_VOID, execve_abort, void)
 LSM_HOOK(int, 0, fs_context_submount, struct fs_context *fc, struct super_block *reference)
 LSM_HOOK(int, 0, fs_context_dup, struct fs_context *fc,
 	 struct fs_context *src_sc)
diff --git a/include/linux/security.h b/include/linux/security.h
index d0eb20f90b26..31532b30c4f0 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -299,6 +299,7 @@ int security_bprm_creds_from_file(struct linux_binprm *bprm, const struct file *
 int security_bprm_check(struct linux_binprm *bprm);
 void security_bprm_committing_creds(const struct linux_binprm *bprm);
 void security_bprm_committed_creds(const struct linux_binprm *bprm);
+void security_execve_abort(void);
 int security_fs_context_submount(struct fs_context *fc, struct super_block *reference);
 int security_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc);
 int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *param);
@@ -648,6 +649,10 @@ static inline void security_bprm_committed_creds(const struct linux_binprm *bprm
 {
 }
 
+static inline void security_execve_abort(void)
+{
+}
+
 static inline int security_fs_context_submount(struct fs_context *fc,
 					   struct super_block *reference)
 {
diff --git a/security/security.c b/security/security.c
index 3aaad75c9ce8..10adc4d3c5e0 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1223,6 +1223,17 @@ void security_bprm_committed_creds(const struct linux_binprm *bprm)
 	call_void_hook(bprm_committed_creds, bprm);
 }
 
+/**
+ * security_execve_abort() - Notify that exec() has failed
+ *
+ * This hook is for undoing changes which cannot be discarded by
+ * abort_creds().
+ */
+void security_execve_abort(void)
+{
+	call_void_hook(execve_abort);
+}
+
 /**
  * security_fs_context_submount() - Initialise fc->security
  * @fc: new filesystem context
-- 
2.18.4

[PATCH v2 2/3] tomoyo: replace current->in_execve flag with security_execve_abort() hook

[Tetsuo Handa]

TOMOYO was using current->in_execve flag in order to restore previous state
when previous execve() request failed. Since security_execve_abort() hook
was added, switch to use it.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 security/tomoyo/tomoyo.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index 04a92c3d65d4..9da11aaffeb9 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -18,34 +18,24 @@ struct tomoyo_domain_info *tomoyo_domain(void)
 {
 	struct tomoyo_task *s = tomoyo_task(current);
 
-	if (s->old_domain_info && !current->in_execve) {
-		atomic_dec(&s->old_domain_info->users);
-		s->old_domain_info = NULL;
-	}
 	return s->domain_info;
 }
 
 /**
- * tomoyo_cred_prepare - Target for security_prepare_creds().
- *
- * @new: Pointer to "struct cred".
- * @old: Pointer to "struct cred".
- * @gfp: Memory allocation flags.
+ * tomoyo_execve_abort - Target for security_execve_abort().
  *
- * Returns 0.
+ * @bprm: void
  */
-static int tomoyo_cred_prepare(struct cred *new, const struct cred *old,
-			       gfp_t gfp)
+static void tomoyo_execve_abort(void)
 {
-	/* Restore old_domain_info saved by previous execve() request. */
+	/* Restore old_domain_info saved by execve() request. */
 	struct tomoyo_task *s = tomoyo_task(current);
 
-	if (s->old_domain_info && !current->in_execve) {
+	if (s->old_domain_info) {
 		atomic_dec(&s->domain_info->users);
 		s->domain_info = s->old_domain_info;
 		s->old_domain_info = NULL;
 	}
-	return 0;
 }
 
 /**
@@ -554,8 +544,8 @@ static const struct lsm_id tomoyo_lsmid = {
  * registering TOMOYO.
  */
 static struct security_hook_list tomoyo_hooks[] __ro_after_init = {
-	LSM_HOOK_INIT(cred_prepare, tomoyo_cred_prepare),
 	LSM_HOOK_INIT(bprm_committed_creds, tomoyo_bprm_committed_creds),
+	LSM_HOOK_INIT(execve_abort, tomoyo_execve_abort),
 	LSM_HOOK_INIT(task_alloc, tomoyo_task_alloc),
 	LSM_HOOK_INIT(task_free, tomoyo_task_free),
 #ifndef CONFIG_SECURITY_TOMOYO_OMIT_USERSPACE_LOADER
-- 
2.18.4

[PATCH v2 3/3] fs/exec: remove current->in_execve flag

[Tetsuo Handa]

Addition of security_execve_abort() hook made it possible to remove
this flag.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 fs/exec.c             | 3 ---
 include/linux/sched.h | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index d6d35a06fd08..c197573b2940 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1865,7 +1865,6 @@ static int bprm_execve(struct linux_binprm *bprm)
 	 * where setuid-ness is evaluated.
 	 */
 	check_unsafe_exec(bprm);
-	current->in_execve = 1;
 	sched_mm_cid_before_execve(current);
 
 	sched_exec();
@@ -1882,7 +1881,6 @@ static int bprm_execve(struct linux_binprm *bprm)
 	sched_mm_cid_after_execve(current);
 	/* execve succeeded */
 	current->fs->in_exec = 0;
-	current->in_execve = 0;
 	rseq_execve(current);
 	user_events_execve(current);
 	acct_update_integrals(current);
@@ -1901,7 +1899,6 @@ static int bprm_execve(struct linux_binprm *bprm)
 
 	sched_mm_cid_after_execve(current);
 	current->fs->in_exec = 0;
-	current->in_execve = 0;
 
 	return retval;
 }
diff --git a/include/linux/sched.h b/include/linux/sched.h
index ffe8f618ab86..66ada87249b1 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -919,9 +919,6 @@ struct task_struct {
 #ifdef CONFIG_RT_MUTEXES
 	unsigned			sched_rt_mutex:1;
 #endif
-
-	/* Bit to tell TOMOYO we're in execve(): */
-	unsigned			in_execve:1;
 	unsigned			in_iowait:1;
 #ifndef TIF_RESTORE_SIGMASK
 	unsigned			restore_sigmask:1;
-- 
2.18.4

Re: [PATCH v2 2/3] tomoyo: replace current->in_execve flag with security_execve_abort() hook

[kernel test robot]

Hi Tetsuo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on kees/for-next/execve]
[also build test WARNING on linux/master tip/sched/core linus/master v6.8-rc2 next-20240202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Tetsuo-Handa/LSM-add-security_execve_abort-hook/20240203-185605
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/execve
patch link:    https://lore.kernel.org/r/2a901d27-dba5-4ff4-9e47-373c54965253%40I-love.SAKURA.ne.jp
patch subject: [PATCH v2 2/3] tomoyo: replace current->in_execve flag with security_execve_abort() hook
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240204/202402040956.6IuNw6B3-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240204/202402040956.6IuNw6B3-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402040956.6IuNw6B3-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> security/tomoyo/tomoyo.c:30: warning: Excess function parameter 'bprm' description in 'tomoyo_execve_abort'


vim +30 security/tomoyo/tomoyo.c

ee18d64c1f6320 David Howells   2009-09-02  23  
0f2a55d5bb2372 Tetsuo Handa    2011-07-14  24  /**
89cebc094231d4 Tetsuo Handa    2024-02-03  25   * tomoyo_execve_abort - Target for security_execve_abort().
0f2a55d5bb2372 Tetsuo Handa    2011-07-14  26   *
89cebc094231d4 Tetsuo Handa    2024-02-03  27   * @bprm: void
0f2a55d5bb2372 Tetsuo Handa    2011-07-14  28   */
89cebc094231d4 Tetsuo Handa    2024-02-03  29  static void tomoyo_execve_abort(void)
f7433243770c77 Kentaro Takeda  2009-02-05 @30  {
89cebc094231d4 Tetsuo Handa    2024-02-03  31  	/* Restore old_domain_info saved by execve() request. */
8c6cb983cd52d7 Tetsuo Handa    2019-01-19  32  	struct tomoyo_task *s = tomoyo_task(current);
43fc460907dc56 Casey Schaufler 2018-09-21  33  
89cebc094231d4 Tetsuo Handa    2024-02-03  34  	if (s->old_domain_info) {
8c6cb983cd52d7 Tetsuo Handa    2019-01-19  35  		atomic_dec(&s->domain_info->users);
8c6cb983cd52d7 Tetsuo Handa    2019-01-19  36  		s->domain_info = s->old_domain_info;
8c6cb983cd52d7 Tetsuo Handa    2019-01-19  37  		s->old_domain_info = NULL;
f7433243770c77 Kentaro Takeda  2009-02-05  38  	}
ec8e6a4e062e2e Tetsuo Handa    2010-02-11  39  }
ec8e6a4e062e2e Tetsuo Handa    2010-02-11  40  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 0/3] fs/exec: remove current->in_execve flag

[Serge Hallyn]

On Sat, Feb 03, 2024 at 07:52:24PM +0900, Tetsuo Handa wrote:
> This is a follow up series for removing current->in_execve flag.
> 
> https://lkml.kernel.org/r/b5a12ecd-468d-4b50-9f8c-17ae2a2560b4@I-love.SAKURA.ne.jp
> 
> [PATCH v2 1/3] LSM: add security_execve_abort() hook
> [PATCH v2 2/3] tomoyo: replace current->in_execve flag with security_execve_abort() hook
> [PATCH v2 3/3] fs/exec: remove current->in_execve flag
> 
>  fs/exec.c                     |    4 +---
>  include/linux/lsm_hook_defs.h |    1 +
>  include/linux/sched.h         |    3 ---
>  include/linux/security.h      |    5 +++++
>  security/security.c           |   11 +++++++++++
>  security/tomoyo/tomoyo.c      |   22 ++++++----------------
>  6 files changed, 24 insertions(+), 22 deletions(-)
> 
> Changes in v2:
> 
>   Replace security_bprm_aborting_creds(const struct linux_binprm *bprm) with
>   security_execve_abort(void), suggested by Eric W. Biederman.

It seems good to me, apart from the mistaken bprm arg mention in
tomoyo_execve_abort() comment in patch 2 which kernel-test-robot found.

Acked-by: Serge E. Hallyn <serge@hallyn.com>

thanks,
-serge

Re: [PATCH v2 1/3] LSM: add security_execve_abort() hook

[Kees Cook]

On Sat, Feb 03, 2024 at 07:52:54PM +0900, Tetsuo Handa wrote:
> A regression caused by commit 978ffcbf00d8 ("execve: open the executable
> 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 flag.
> 
> 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 for
> emulating security_bprm_free() hook which has been removed by introduction
> 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: Revive
> 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() calls,
> and TOMOYO can stop carrying state if a hook for restoring previous state
> upon failed execve() call were provided, this patch revives the hook.
> 
> Since security_bprm_committing_creds() and security_bprm_committed_creds()
> hooks are called when an execve() request succeeded, we don't need to call
> 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?

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH v2 2/3] tomoyo: replace current->in_execve flag with security_execve_abort() hook

[Kees Cook]

On Sat, Feb 03, 2024 at 07:53:17PM +0900, Tetsuo Handa wrote:
> TOMOYO was using current->in_execve flag in order to restore previous state
> when previous execve() request failed. Since security_execve_abort() hook
> was added, switch to use it.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

With the kern-doc fixed, this looks good. (I can fix up the kern-doc if
this goes via my tree.)

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH v2 3/3] fs/exec: remove current->in_execve flag

[Kees Cook]

On Sat, Feb 03, 2024 at 07:53:39PM +0900, Tetsuo Handa wrote:
> Addition of security_execve_abort() hook made it possible to remove
> this flag.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Yay removal! :)

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH v2 1/3] LSM: add security_execve_abort() hook

[Tetsuo Handa]

On 2024/02/07 23:24, Kees Cook wrote:
> 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?

Yes, please do so. (My git tree is currently down.)

> 
> There's already an Ack from Serge, so this seems a reasonable way to go
> unless Paul would like it done some other way?
> 
> Reviewed-by: Kees Cook <keescook@chromium.org>
> 

Thank you.

Re: [PATCH v2 1/3] LSM: add security_execve_abort() hook

[Paul Moore]

On Wed, Feb 7, 2024 at 9:24 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Sat, Feb 03, 2024 at 07:52:54PM +0900, Tetsuo Handa wrote:
> > A regression caused by commit 978ffcbf00d8 ("execve: open the executable
> > 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 flag.
> >
> > 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 for
> > emulating security_bprm_free() hook which has been removed by introduction
> > 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: Revive
> > 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() calls,
> > and TOMOYO can stop carrying state if a hook for restoring previous state
> > upon failed execve() call were provided, this patch revives the hook.
> >
> > Since security_bprm_committing_creds() and security_bprm_committed_creds()
> > hooks are called when an execve() request succeeded, we don't need to call
> > 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

Re: [PATCH v2 1/3] LSM: add security_execve_abort() hook

[Kees Cook]

On Wed, Feb 07, 2024 at 10:21:07AM -0500, Paul Moore wrote:
> On Wed, Feb 7, 2024 at 9:24 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Sat, Feb 03, 2024 at 07:52:54PM +0900, Tetsuo Handa wrote:
> > > A regression caused by commit 978ffcbf00d8 ("execve: open the executable
> > > 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 flag.
> > >
> > > 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 for
> > > emulating security_bprm_free() hook which has been removed by introduction
> > > 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: Revive
> > > 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() calls,
> > > and TOMOYO can stop carrying state if a hook for restoring previous state
> > > upon failed execve() call were provided, this patch revives the hook.
> > >
> > > Since security_bprm_committing_creds() and security_bprm_committed_creds()
> > > hooks are called when an execve() request succeeded, we don't need to call
> > > 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?

My understanding was that TOMOYO is currently broken in Linus's tree. If
that's true, I'd like to make sure it gets fixed before v6.8 is
released.

If it's working okay, then sure, that's fine to wait. :)

-Kees

-- 
Kees Cook

Re: [PATCH v2 1/3] LSM: add security_execve_abort() hook

[Paul Moore]

On Wed, Feb 7, 2024 at 10:43 AM Kees Cook <keescook@chromium.org> wrote:
> On Wed, Feb 07, 2024 at 10:21:07AM -0500, Paul Moore wrote:

...

> > 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?
>
> My understanding was that TOMOYO is currently broken in Linus's tree. If
> that's true, I'd like to make sure it gets fixed before v6.8 is
> released.
>
> If it's working okay, then sure, that's fine to wait. :)

Okay, let's get confirmation from Tetsuo on the current state of
TOMOYO in Linus' tree.  If it is currently broken, I'll merge the next
updated patchset from Tetsuo into the lsm/stable-6.8 branch and send
it up to Linus during v6.8-rcX after some soaking in linux-next.  If
it's working, we'll wait :)

-- 
paul-moore.com

Re: [PATCH v2 1/3] LSM: add security_execve_abort() hook

[Linus Torvalds]

On Wed, 7 Feb 2024 at 16:45, Paul Moore <paul@paul-moore.com> wrote:
>
> Okay, let's get confirmation from Tetsuo on the current state of
> TOMOYO in Linus' tree.  If it is currently broken [..]

As far as I understand, the current state is working, just the horrid
random flag.

So I think the series is a cleanup and worth doing, but also not
hugely urgent. But it would probably be good to just get this whole
thing over and done with, rather than leave it lingering for another
release for no reason.

                Linus

Re: [PATCH v2 1/3] LSM: add security_execve_abort() hook

[Paul Moore]

On Wed, Feb 7, 2024 at 12:57 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, 7 Feb 2024 at 16:45, Paul Moore <paul@paul-moore.com> wrote:
> >
> > Okay, let's get confirmation from Tetsuo on the current state of
> > TOMOYO in Linus' tree.  If it is currently broken [..]
>
> As far as I understand, the current state is working, just the horrid
> random flag.
>
> So I think the series is a cleanup and worth doing, but also not
> hugely urgent. But it would probably be good to just get this whole
> thing over and done with, rather than leave it lingering for another
> release for no reason.

I've always operated under a policy of "just fixes" during the -rcX
period of development, but you're the boss.  Once we get something
suitable for merging I'll send it up to you after it has soaked in
linux-next for a bit.

-- 
paul-moore.com

Re: [PATCH v2 1/3] LSM: add security_execve_abort() hook

[Tetsuo Handa]

On 2024/02/08 2:57, Linus Torvalds wrote:
> On Wed, 7 Feb 2024 at 16:45, Paul Moore <paul@paul-moore.com> wrote:
>>
>> Okay, let's get confirmation from Tetsuo on the current state of
>> TOMOYO in Linus' tree.  If it is currently broken [..]
> 
> As far as I understand, the current state is working, just the horrid
> random flag.

Yes, the current state is working.

> 
> So I think the series is a cleanup and worth doing, but also not
> hugely urgent. But it would probably be good to just get this whole
> thing over and done with, rather than leave it lingering for another
> release for no reason.

Right.

Re: [PATCH v2 1/3] LSM: add security_execve_abort() hook

[Paul Moore]

On Wed, Feb 7, 2024 at 5:23 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> On 2024/02/08 2:57, Linus Torvalds wrote:
> > On Wed, 7 Feb 2024 at 16:45, Paul Moore <paul@paul-moore.com> wrote:
> >>
> >> Okay, let's get confirmation from Tetsuo on the current state of
> >> TOMOYO in Linus' tree.  If it is currently broken [..]
> >
> > As far as I understand, the current state is working, just the horrid
> > random flag.
>
> Yes, the current state is working.

Thanks for confirming that Tetsuo.

-- 
paul-moore.com