diff mbox series

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

Message ID 2a901d27-dba5-4ff4-9e47-373c54965253@I-love.SAKURA.ne.jp
State New
Headers
Series fs/exec: remove current->in_execve flag |

Commit Message

Tetsuo Handa Feb. 3, 2024, 10:53 a.m. UTC 
  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(-)

Comments

kernel test robot Feb. 4, 2024, 1:53 a.m. UTC | #1 
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
Kees Cook Feb. 7, 2024, 2:25 p.m. UTC | #2 
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>
diff mbox series

Patch

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