Linux Security Modules development
 help / color / mirror / Atom feed
* [PATCH] security: provide an inlined static branch for security_inode_permission()
From: Mateusz Guzik @ 2025-11-09 19:29 UTC (permalink / raw)
  To: paul
  Cc: casey, keescook, song, andrii, kpsingh, linux-security-module,
	linux-fsdevel, Mateusz Guzik

The routine is executing for every path component during name resolution in
vfs and shows up on the profile to the tune of 2% of CPU time in my
tests.

The only LSMs which install hoooks there are selinux and smack, meaning
most installs don't have it and this ends up being a call to do nothing.

While perhaps a more generic mechanism covering all hoooks would be
preferred, I implemented a bare minimum version which gets out of the
way for my needs.

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 include/linux/security.h | 11 ++++++++++-
 security/security.c      | 12 ++++++++++--
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 92ac3f27b973..0ce1d73167ed 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -68,6 +68,8 @@ struct watch;
 struct watch_notification;
 struct lsm_ctx;
 
+DECLARE_STATIC_KEY_FALSE(security_inode_permission_has_hooks);
+
 /* Default (no) options for the capable function */
 #define CAP_OPT_NONE 0x0
 /* If capable should audit the security request */
@@ -421,7 +423,14 @@ int security_inode_rename(struct inode *old_dir, struct dentry *old_dentry,
 int security_inode_readlink(struct dentry *dentry);
 int security_inode_follow_link(struct dentry *dentry, struct inode *inode,
 			       bool rcu);
-int security_inode_permission(struct inode *inode, int mask);
+int __security_inode_permission(struct inode *inode, int mask);
+static inline int security_inode_permission(struct inode *inode, int mask)
+{
+	if (static_branch_unlikely(&security_inode_permission_has_hooks))
+		return __security_inode_permission(inode, mask);
+	return 0;
+}
+
 int security_inode_setattr(struct mnt_idmap *idmap,
 			   struct dentry *dentry, struct iattr *attr);
 void security_inode_post_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
diff --git a/security/security.c b/security/security.c
index 4d3c03a4524c..e879f034a77c 100644
--- a/security/security.c
+++ b/security/security.c
@@ -51,6 +51,8 @@ do {						\
 
 #define LSM_DEFINE_UNROLL(M, ...) UNROLL(MAX_LSM_COUNT, M, __VA_ARGS__)
 
+DEFINE_STATIC_KEY_FALSE(security_inode_permission_has_hooks);
+
 /*
  * These are descriptions of the reasons that can be passed to the
  * security_locked_down() LSM hook. Placing this array here allows
@@ -639,6 +641,12 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
 		lsm_static_call_init(&hooks[i]);
 	}
 
+	if (static_key_enabled(&static_calls_table.inode_permission->active->key)) {
+		if (!static_key_enabled(&security_inode_permission_has_hooks.key)) {
+			static_branch_enable(&security_inode_permission_has_hooks);
+		}
+	}
+
 	/*
 	 * Don't try to append during early_security_init(), we'll come back
 	 * and fix this up afterwards.
@@ -2343,7 +2351,7 @@ int security_inode_follow_link(struct dentry *dentry, struct inode *inode,
 }
 
 /**
- * security_inode_permission() - Check if accessing an inode is allowed
+ * __security_inode_permission() - Check if accessing an inode is allowed
  * @inode: inode
  * @mask: access mask
  *
@@ -2356,7 +2364,7 @@ int security_inode_follow_link(struct dentry *dentry, struct inode *inode,
  *
  * Return: Returns 0 if permission is granted.
  */
-int security_inode_permission(struct inode *inode, int mask)
+int __security_inode_permission(struct inode *inode, int mask)
 {
 	if (unlikely(IS_PRIVATE(inode)))
 		return 0;
-- 
2.48.1


^ permalink raw reply related

* [RFC PATCH 3/3] ptrace: ensure PTRACE_EVENT_EXIT won't stop if the tracee is killed by exec
From: Oleg Nesterov @ 2025-11-09 17:16 UTC (permalink / raw)
  To: Bernd Edlinger, Linus Torvalds, Dmitry Levin
  Cc: Alexander Viro, Alexey Dobriyan, Kees Cook, Andy Lutomirski,
	Will Drewry, Christian Brauner, Andrew Morton, Michal Hocko,
	Serge Hallyn, James Morris, Randy Dunlap, Suren Baghdasaryan,
	Yafang Shao, Helge Deller, Eric W. Biederman, Adrian Reber,
	Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kselftest, linux-mm, linux-security-module, tiozhang,
	Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
	Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
	Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
	David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
	David Windsor, Mateusz Guzik, Ard Biesheuvel,
	Joel Fernandes (Google), Matthew Wilcox (Oracle),
	Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
	Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
	Eric Dumazet
In-Reply-To: <aRDL3HOB21pMVMWC@redhat.com>

The previous patch fixed the deadlock when mt-exec waits for debugger
which should reap a zombie thread, but we can hit the same problem if
the killed sub-thread stops in ptrace_event(PTRACE_EVENT_EXIT). Change
ptrace_stop() to check signal->group_exit_task.

This is a user-visible change. But hopefully it can't break anything.
Note that the semantics of PTRACE_EVENT_EXIT was never really defined,
it depends on /dev/random. Just for example, currently a sub-thread
killed by exec will stop, but if it exits on its own and races with
exec it will not stop, so nobody can rely on PTRACE_EVENT_EXIT anyway.

We really need to finally define what PTRACE_EVENT_EXIT should actually
do, but this needs other changes.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/signal.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/signal.c b/kernel/signal.c
index 334212044940..59f61e07905b 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2376,6 +2376,10 @@ static int ptrace_stop(int exit_code, int why, unsigned long message,
 	if (!current->ptrace || __fatal_signal_pending(current))
 		return exit_code;
 
+	/* de_thread() -> wait_for_notify_count() waits for us */
+	if (current->signal->group_exec_task)
+		return exit_code;
+
 	set_special_state(TASK_TRACED);
 	current->jobctl |= JOBCTL_TRACED;
 
-- 
2.25.1.362.g51ebf55



^ permalink raw reply related

* [RFC PATCH 2/3] exec: don't wait for zombie threads with cred_guard_mutex held
From: Oleg Nesterov @ 2025-11-09 17:15 UTC (permalink / raw)
  To: Bernd Edlinger, Linus Torvalds, Dmitry Levin
  Cc: Alexander Viro, Alexey Dobriyan, Kees Cook, Andy Lutomirski,
	Will Drewry, Christian Brauner, Andrew Morton, Michal Hocko,
	Serge Hallyn, James Morris, Randy Dunlap, Suren Baghdasaryan,
	Yafang Shao, Helge Deller, Eric W. Biederman, Adrian Reber,
	Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kselftest, linux-mm, linux-security-module, tiozhang,
	Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
	Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
	Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
	David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
	David Windsor, Mateusz Guzik, Ard Biesheuvel,
	Joel Fernandes (Google), Matthew Wilcox (Oracle),
	Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
	Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
	Eric Dumazet
In-Reply-To: <aRDL3HOB21pMVMWC@redhat.com>

This simple program

    #include <unistd.h>
    #include <signal.h>
    #include <sys/ptrace.h>
    #include <pthread.h>

    void *thread(void *arg)
    {
	    ptrace(PTRACE_TRACEME, 0,0,0);
	    return NULL;
    }

    int main(void)
    {
	    int pid = fork();

	    if (!pid) {
		    pthread_t pt;
		    pthread_create(&pt, NULL, thread, NULL);
		    pthread_join(pt, NULL);
		    execlp("echo", "echo", "passed", NULL);
	    }

	    sleep(1);
	    ptrace(PTRACE_ATTACH, pid, 0,0);
	    kill(pid, SIGCONT);

	    return 0;
    }

hangs because de_thread() waits for debugger which should release the killed
thread with cred_guard_mutex held, while the debugger sleeps waiting for the
same mutex. Not really that bad, the tracer can be killed, but still this is
a bug and people hit it in practice.

With this patch:

	- de_thread() waits until all the sub-threads pass exit_notify() and
	  become zombies.

	- setup_new_exec() waits until all the sub-threads are reaped without
	  cred_guard_mutex held.

	- unshare_sighand() and flush_signal_handlers() are moved from
	  begin_new_exec() to setup_new_exec(), we can't call them until all
	  sub-threads go away.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 fs/exec.c       | 140 +++++++++++++++++++++++-------------------------
 kernel/exit.c   |   9 ++--
 kernel/signal.c |   2 +-
 3 files changed, 71 insertions(+), 80 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 136a7ab5d91c..2bac7deb9a98 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -905,42 +905,56 @@ static int exec_mmap(struct mm_struct *mm)
 	return 0;
 }
 
-static int de_thread(struct task_struct *tsk)
+static int kill_sub_threads(struct task_struct *tsk)
 {
 	struct signal_struct *sig = tsk->signal;
-	struct sighand_struct *oldsighand = tsk->sighand;
-	spinlock_t *lock = &oldsighand->siglock;
-
-	if (thread_group_empty(tsk))
-		goto no_thread_group;
+	int err = -EINTR;
 
-	/*
-	 * Kill all other threads in the thread group.
-	 */
-	spin_lock_irq(lock);
-	if ((sig->flags & SIGNAL_GROUP_EXIT) || sig->group_exec_task) {
-		/*
-		 * Another group action in progress, just
-		 * return so that the signal is processed.
-		 */
-		spin_unlock_irq(lock);
-		return -EAGAIN;
+	read_lock(&tasklist_lock);
+	spin_lock_irq(&tsk->sighand->siglock);
+	if (!((sig->flags & SIGNAL_GROUP_EXIT) || sig->group_exec_task)) {
+		sig->group_exec_task = tsk;
+		sig->notify_count = -zap_other_threads(tsk);
+		err = 0;
 	}
+	spin_unlock_irq(&tsk->sighand->siglock);
+	read_unlock(&tasklist_lock);
 
-	sig->group_exec_task = tsk;
-	sig->notify_count = zap_other_threads(tsk);
-	if (!thread_group_leader(tsk))
-		sig->notify_count--;
+	return err;
+}
 
-	while (sig->notify_count) {
-		__set_current_state(TASK_KILLABLE);
-		spin_unlock_irq(lock);
-		schedule();
+static int wait_for_notify_count(struct task_struct *tsk)
+{
+	for (;;) {
 		if (__fatal_signal_pending(tsk))
-			goto killed;
-		spin_lock_irq(lock);
+			return -EINTR;
+		set_current_state(TASK_KILLABLE);
+		if (!tsk->signal->notify_count)
+			break;
+		schedule();
 	}
-	spin_unlock_irq(lock);
+	__set_current_state(TASK_RUNNING);
+	return 0;
+}
+
+static void clear_group_exec_task(struct task_struct *tsk)
+{
+	struct signal_struct *sig = tsk->signal;
+
+	/* protects against exit_notify() and __exit_signal() */
+	read_lock(&tasklist_lock);
+	sig->group_exec_task = NULL;
+	sig->notify_count = 0;
+	read_unlock(&tasklist_lock);
+}
+
+static int de_thread(struct task_struct *tsk)
+{
+	if (thread_group_empty(tsk))
+		goto no_thread_group;
+
+	if (kill_sub_threads(tsk) || wait_for_notify_count(tsk))
+		return -EINTR;
 
 	/*
 	 * At this point all other threads have exited, all we have to
@@ -948,26 +962,10 @@ static int de_thread(struct task_struct *tsk)
 	 * and to assume its PID:
 	 */
 	if (!thread_group_leader(tsk)) {
-		struct task_struct *leader = tsk->group_leader;
-
-		for (;;) {
-			cgroup_threadgroup_change_begin(tsk);
-			write_lock_irq(&tasklist_lock);
-			/*
-			 * Do this under tasklist_lock to ensure that
-			 * exit_notify() can't miss ->group_exec_task
-			 */
-			sig->notify_count = -1;
-			if (likely(leader->exit_state))
-				break;
-			__set_current_state(TASK_KILLABLE);
-			write_unlock_irq(&tasklist_lock);
-			cgroup_threadgroup_change_end(tsk);
-			schedule();
-			if (__fatal_signal_pending(tsk))
-				goto killed;
-		}
+		struct task_struct *leader = tsk->group_leader, *t;
 
+		cgroup_threadgroup_change_begin(tsk);
+		write_lock_irq(&tasklist_lock);
 		/*
 		 * The only record we have of the real-time age of a
 		 * process, regardless of execs it's done, is start_time.
@@ -1000,8 +998,8 @@ static int de_thread(struct task_struct *tsk)
 		list_replace_rcu(&leader->tasks, &tsk->tasks);
 		list_replace_init(&leader->sibling, &tsk->sibling);
 
-		tsk->group_leader = tsk;
-		leader->group_leader = tsk;
+		for_each_thread(tsk, t)
+			t->group_leader = tsk;
 
 		tsk->exit_signal = SIGCHLD;
 		leader->exit_signal = -1;
@@ -1021,23 +1019,11 @@ static int de_thread(struct task_struct *tsk)
 		release_task(leader);
 	}
 
-	sig->group_exec_task = NULL;
-	sig->notify_count = 0;
-
 no_thread_group:
 	/* we have changed execution domain */
 	tsk->exit_signal = SIGCHLD;
-
 	BUG_ON(!thread_group_leader(tsk));
 	return 0;
-
-killed:
-	/* protects against exit_notify() and __exit_signal() */
-	read_lock(&tasklist_lock);
-	sig->group_exec_task = NULL;
-	sig->notify_count = 0;
-	read_unlock(&tasklist_lock);
-	return -EAGAIN;
 }
 
 
@@ -1171,13 +1157,6 @@ int begin_new_exec(struct linux_binprm * bprm)
 	flush_itimer_signals();
 #endif
 
-	/*
-	 * Make the signal table private.
-	 */
-	retval = unshare_sighand(me);
-	if (retval)
-		goto out_unlock;
-
 	me->flags &= ~(PF_RANDOMIZE | PF_FORKNOEXEC |
 					PF_NOFREEZE | PF_NO_SETAFFINITY);
 	flush_thread();
@@ -1249,7 +1228,6 @@ int begin_new_exec(struct linux_binprm * bprm)
 	/* An exec changes our domain. We are no longer part of the thread
 	   group */
 	WRITE_ONCE(me->self_exec_id, me->self_exec_id + 1);
-	flush_signal_handlers(me, 0);
 
 	retval = set_cred_ucounts(bprm->cred);
 	if (retval < 0)
@@ -1293,8 +1271,9 @@ int begin_new_exec(struct linux_binprm * bprm)
 	up_write(&me->signal->exec_update_lock);
 	if (!bprm->cred)
 		mutex_unlock(&me->signal->cred_guard_mutex);
-
 out:
+	if (me->signal->group_exec_task == me)
+		clear_group_exec_task(me);
 	return retval;
 }
 EXPORT_SYMBOL(begin_new_exec);
@@ -1325,6 +1304,8 @@ int setup_new_exec(struct linux_binprm * bprm)
 {
 	/* Setup things that can depend upon the personality */
 	struct task_struct *me = current;
+	struct signal_struct *sig = me->signal;
+	int err = 0;
 
 	arch_pick_mmap_layout(me->mm, &bprm->rlim_stack);
 
@@ -1335,10 +1316,23 @@ int setup_new_exec(struct linux_binprm * bprm)
 	 * some architectures like powerpc
 	 */
 	me->mm->task_size = TASK_SIZE;
-	up_write(&me->signal->exec_update_lock);
-	mutex_unlock(&me->signal->cred_guard_mutex);
+	up_write(&sig->exec_update_lock);
+	mutex_unlock(&sig->cred_guard_mutex);
 
-	return 0;
+	if (sig->group_exec_task) {
+		spin_lock_irq(&me->sighand->siglock);
+		sig->notify_count = sig->nr_threads - 1;
+		spin_unlock_irq(&me->sighand->siglock);
+
+		err = wait_for_notify_count(me);
+		clear_group_exec_task(me);
+	}
+
+	if (!err)
+		err = unshare_sighand(me);
+	if (!err)
+		flush_signal_handlers(me, 0);
+	return err;
 }
 EXPORT_SYMBOL(setup_new_exec);
 
diff --git a/kernel/exit.c b/kernel/exit.c
index f041f0c05ebb..bcde78c97253 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -178,10 +178,7 @@ static void __exit_signal(struct release_task_post *post, struct task_struct *ts
 		tty = sig->tty;
 		sig->tty = NULL;
 	} else {
-		/*
-		 * If there is any task waiting for the group exit
-		 * then notify it:
-		 */
+		/* mt-exec, setup_new_exec() -> wait_for_notify_count() */
 		if (sig->notify_count > 0 && !--sig->notify_count)
 			wake_up_process(sig->group_exec_task);
 
@@ -766,8 +763,8 @@ static void exit_notify(struct task_struct *tsk, int group_dead)
 		list_add(&tsk->ptrace_entry, &dead);
 	}
 
-	/* mt-exec, de_thread() is waiting for group leader */
-	if (unlikely(tsk->signal->notify_count < 0))
+	/* mt-exec, de_thread() -> wait_for_notify_count() */
+	if (tsk->signal->notify_count < 0 && !++tsk->signal->notify_count)
 		wake_up_process(tsk->signal->group_exec_task);
 	write_unlock_irq(&tasklist_lock);
 
diff --git a/kernel/signal.c b/kernel/signal.c
index fe9190d84f28..334212044940 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1343,13 +1343,13 @@ int zap_other_threads(struct task_struct *p)
 
 	for_other_threads(p, t) {
 		task_clear_jobctl_pending(t, JOBCTL_PENDING_MASK);
-		count++;
 
 		/* Don't bother with already dead threads */
 		if (t->exit_state)
 			continue;
 		sigaddset(&t->pending.signal, SIGKILL);
 		signal_wake_up(t, 1);
+		count++;
 	}
 
 	return count;
-- 
2.25.1.362.g51ebf55



^ permalink raw reply related

* [RFC PATCH 1/3] exec: make setup_new_exec() return int
From: Oleg Nesterov @ 2025-11-09 17:14 UTC (permalink / raw)
  To: Bernd Edlinger, Linus Torvalds, Dmitry Levin
  Cc: Alexander Viro, Alexey Dobriyan, Kees Cook, Andy Lutomirski,
	Will Drewry, Christian Brauner, Andrew Morton, Michal Hocko,
	Serge Hallyn, James Morris, Randy Dunlap, Suren Baghdasaryan,
	Yafang Shao, Helge Deller, Eric W. Biederman, Adrian Reber,
	Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kselftest, linux-mm, linux-security-module, tiozhang,
	Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
	Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
	Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
	David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
	David Windsor, Mateusz Guzik, Ard Biesheuvel,
	Joel Fernandes (Google), Matthew Wilcox (Oracle),
	Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
	Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
	Eric Dumazet
In-Reply-To: <aRDL3HOB21pMVMWC@redhat.com>

Preparation. After the next change setup_new_exec() can fail.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 fs/binfmt_elf.c         | 4 +++-
 fs/binfmt_elf_fdpic.c   | 4 +++-
 fs/binfmt_flat.c        | 4 +++-
 fs/exec.c               | 4 +++-
 include/linux/binfmts.h | 2 +-
 5 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index e4653bb99946..8a38689ae4b0 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1021,7 +1021,9 @@ static int load_elf_binary(struct linux_binprm *bprm)
 	if (!(current->personality & ADDR_NO_RANDOMIZE) && snapshot_randomize_va_space)
 		current->flags |= PF_RANDOMIZE;
 
-	setup_new_exec(bprm);
+	retval = setup_new_exec(bprm);
+	if (retval)
+		goto out_free_dentry;
 
 	/* Do this so that we can load the interpreter, if need be.  We will
 	   change some of these later */
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 48fd2de3bca0..7ad98b8132fc 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -351,7 +351,9 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
 	if (elf_read_implies_exec(&exec_params.hdr, executable_stack))
 		current->personality |= READ_IMPLIES_EXEC;
 
-	setup_new_exec(bprm);
+	retval = setup_new_exec(bprm);
+	if (retval)
+		goto error;
 
 	set_binfmt(&elf_fdpic_format);
 
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index b5b5ca1a44f7..78e9ca128ea7 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -512,7 +512,9 @@ static int load_flat_file(struct linux_binprm *bprm,
 
 	/* OK, This is the point of no return */
 	set_personality(PER_LINUX_32BIT);
-	setup_new_exec(bprm);
+	ret = setup_new_exec(bprm);
+	if (ret)
+		goto err;
 
 	/*
 	 * calculate the extra space we need to map in
diff --git a/fs/exec.c b/fs/exec.c
index 6b70c6726d31..136a7ab5d91c 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1321,7 +1321,7 @@ void would_dump(struct linux_binprm *bprm, struct file *file)
 }
 EXPORT_SYMBOL(would_dump);
 
-void setup_new_exec(struct linux_binprm * bprm)
+int setup_new_exec(struct linux_binprm * bprm)
 {
 	/* Setup things that can depend upon the personality */
 	struct task_struct *me = current;
@@ -1337,6 +1337,8 @@ void setup_new_exec(struct linux_binprm * bprm)
 	me->mm->task_size = TASK_SIZE;
 	up_write(&me->signal->exec_update_lock);
 	mutex_unlock(&me->signal->cred_guard_mutex);
+
+	return 0;
 }
 EXPORT_SYMBOL(setup_new_exec);
 
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 65abd5ab8836..678b7525ac5a 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -123,7 +123,7 @@ extern void unregister_binfmt(struct linux_binfmt *);
 
 extern int __must_check remove_arg_zero(struct linux_binprm *);
 extern int begin_new_exec(struct linux_binprm * bprm);
-extern void setup_new_exec(struct linux_binprm * bprm);
+extern int setup_new_exec(struct linux_binprm * bprm);
 extern void finalize_exec(struct linux_binprm *bprm);
 extern void would_dump(struct linux_binprm *, struct file *);
 
-- 
2.25.1.362.g51ebf55



^ permalink raw reply related

* [RFC PATCH 0/3] mt-exec: fix deadlock with ptrace_attach()
From: Oleg Nesterov @ 2025-11-09 17:14 UTC (permalink / raw)
  To: Bernd Edlinger, Linus Torvalds, Dmitry Levin
  Cc: Alexander Viro, Alexey Dobriyan, Kees Cook, Andy Lutomirski,
	Will Drewry, Christian Brauner, Andrew Morton, Michal Hocko,
	Serge Hallyn, James Morris, Randy Dunlap, Suren Baghdasaryan,
	Yafang Shao, Helge Deller, Eric W. Biederman, Adrian Reber,
	Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kselftest, linux-mm, linux-security-module, tiozhang,
	Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
	Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
	Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
	David Hildenbrand, Dave Chinner, Shuah Khan, Elena Reshetova,
	David Windsor, Mateusz Guzik, Ard Biesheuvel,
	Joel Fernandes (Google), Matthew Wilcox (Oracle),
	Hans Liljestrand, Penglei Jiang, Lorenzo Stoakes, Adrian Ratiu,
	Ingo Molnar, Peter Zijlstra (Intel), Cyrill Gorcunov,
	Eric Dumazet
In-Reply-To: <GV2PPF74270EBEE9EF78827D73D3D7212F7E432A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>

Not for inclusion yet. 2/2 is untested, incomplete, possibly buggy.

But could you review at least the intent? Do you see any problem with
this approach?

This problem is very, very old. It seems that nobody can suggest a
simple/clean fix...

Oleg.
---

 fs/binfmt_elf.c         |   4 +-
 fs/binfmt_elf_fdpic.c   |   4 +-
 fs/binfmt_flat.c        |   4 +-
 fs/exec.c               | 142 +++++++++++++++++++++++-------------------------
 include/linux/binfmts.h |   2 +-
 kernel/exit.c           |   9 +--
 kernel/signal.c         |   6 +-
 7 files changed, 87 insertions(+), 84 deletions(-)


^ permalink raw reply

* Re: [syzbot] [fs?] WARNING in destroy_super_work
From: syzbot @ 2025-11-09 14:11 UTC (permalink / raw)
  To: akpm, anna-maria, bpf, brauner, bsegall, cgroups, david,
	dietmar.eggemann, frederic, hannes, jack, jsavitz, juri.lelli,
	kees, liam.howlett, linux-fsdevel, linux-kernel, linux-mm,
	linux-security-module, lorenzo.stoakes, mgorman, mhocko, mingo,
	mjguzik, mkoutny, oleg, paul, peterz, rostedt, rppt, sergeh,
	surenb, syzkaller-bugs, tglx, tj, vbabka, vincent.guittot, viro,
	vschneid
In-Reply-To: <20251109-lesung-erkaufen-476f6fb00b1b@brauner>

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+1957b26299cf3ff7890c@syzkaller.appspotmail.com
Tested-by: syzbot+1957b26299cf3ff7890c@syzkaller.appspotmail.com

Tested on:

commit:         241462cd ns: fixes for namespace iteration and active ..
git tree:       https://github.com/brauner/linux.git namespace-6.19.fixes
console output: https://syzkaller.appspot.com/x/log.txt?x=11e1517c580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=f1b1a45727d1f117
dashboard link: https://syzkaller.appspot.com/bug?extid=1957b26299cf3ff7890c
compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8

Note: no patches were applied.
Note: testing is done by a robot and is best-effort only.

^ permalink raw reply

* Re: [syzbot] [fs?] WARNING in destroy_super_work
From: Christian Brauner @ 2025-11-09 13:39 UTC (permalink / raw)
  To: syzbot
  Cc: Liam.Howlett, akpm, anna-maria, bpf, bsegall, cgroups, david,
	dietmar.eggemann, frederic, hannes, jack, jsavitz, juri.lelli,
	kees, linux-fsdevel, linux-kernel, linux-mm,
	linux-security-module, lorenzo.stoakes, mgorman, mhocko, mingo,
	mjguzik, mkoutny, oleg, paul, peterz, rostedt, rppt, sergeh,
	surenb, syzkaller-bugs, tglx, tj, vbabka, vincent.guittot, viro,
	vschneid
In-Reply-To: <690da04f.a70a0220.22f260.0027.GAE@google.com>

On Thu, Nov 06, 2025 at 11:31:27PM -0800, syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    982312090977 Add linux-next specific files for 20251103
> git tree:       linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=17b2932f980000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=43cc0e31558cb527
> dashboard link: https://syzkaller.appspot.com/bug?extid=1957b26299cf3ff7890c
> compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1347817c580000
> 
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/40058f8a830c/disk-98231209.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/1d7f42e8639f/vmlinux-98231209.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/d8bb0284f393/bzImage-98231209.xz
> 
> The issue was bisected to:
> 
> commit 3c9820d5c64aeaadea7ffe3a6bb99d019a5ff46a
> Author: Christian Brauner <brauner@kernel.org>
> Date:   Wed Oct 29 12:20:24 2025 +0000
> 
>     ns: add active reference count
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=101e9bcd980000
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=121e9bcd980000
> console output: https://syzkaller.appspot.com/x/log.txt?x=141e9bcd980000
> 
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+1957b26299cf3ff7890c@syzkaller.appspotmail.com
> Fixes: 3c9820d5c64a ("ns: add active reference count")

#syz test: https://github.com/brauner/linux.git namespace-6.19.fixes

^ permalink raw reply

* Re: [syzbot] [fs?] WARNING in nsproxy_ns_active_put
From: syzbot @ 2025-11-09  8:24 UTC (permalink / raw)
  To: Liam.Howlett, Liam.Howlett, akpm, bpf, brauner, bsegall, david,
	dietmar.eggemann, jack, jsavitz, juri.lelli, kartikey406, kees,
	linux-fsdevel, linux-kernel, linux-mm, linux-security-module,
	lorenzo.stoakes, mgorman, mhocko, mingo, mjguzik, oleg, paul,
	peterz, rostedt, rppt, sergeh, surenb, syzkaller-bugs, vbabka,
	vincent.guittot, viro, vschneid
In-Reply-To: <690bfb9e.050a0220.2e3c35.0013.GAE@google.com>

syzbot has bisected this issue to:

commit 3a18f809184bc5a1cfad7cde5b8b026e2ff61587
Author: Christian Brauner <brauner@kernel.org>
Date:   Wed Oct 29 12:20:24 2025 +0000

    ns: add active reference count

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=11a350b4580000
start commit:   9c0826a5d9aa Add linux-next specific files for 20251107
git tree:       linux-next
final oops:     https://syzkaller.appspot.com/x/report.txt?x=13a350b4580000
console output: https://syzkaller.appspot.com/x/log.txt?x=15a350b4580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=f2ebeee52bf052b8
dashboard link: https://syzkaller.appspot.com/bug?extid=0b2e79f91ff6579bfa5b
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1639d084580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1625aa92580000

Reported-by: syzbot+0b2e79f91ff6579bfa5b@syzkaller.appspotmail.com
Fixes: 3a18f809184b ("ns: add active reference count")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

^ permalink raw reply

* Re: [PATCH] KEYS: encrypted: Return early on allocation failure and drop goto
From: Jarkko Sakkinen @ 2025-11-09  4:47 UTC (permalink / raw)
  To: Thorsten Blum
  Cc: Mimi Zohar, David Howells, Paul Moore, James Morris,
	Serge E. Hallyn, linux-integrity, keyrings, linux-security-module,
	linux-kernel
In-Reply-To: <20251029163157.119000-1-thorsten.blum@linux.dev>

On Wed, Oct 29, 2025 at 05:31:56PM +0100, Thorsten Blum wrote:
> Return ERR_PTR(-ENOMEM) immediately if memory allocation fails, instead
> of using goto and returning a NULL pointer, and remove the now-unused
> 'out' label.
> 
> At the call site, check 'ascii_buf' with IS_ERR() and propagate the
> error code returned by datablob_format().
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  security/keys/encrypted-keys/encrypted.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index be1f2118447c..25df00b7dbe9 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -276,7 +276,7 @@ static char *datablob_format(struct encrypted_key_payload *epayload,
>  
>  	ascii_buf = kmalloc(asciiblob_len + 1, GFP_KERNEL);
>  	if (!ascii_buf)
> -		goto out;
> +		return ERR_PTR(-ENOMEM);
>  
>  	ascii_buf[asciiblob_len] = '\0';
>  
> @@ -288,7 +288,6 @@ static char *datablob_format(struct encrypted_key_payload *epayload,
>  	bufp = &ascii_buf[len];
>  	for (i = 0; i < (asciiblob_len - len) / 2; i++)
>  		bufp = hex_byte_pack(bufp, iv[i]);
> -out:
>  	return ascii_buf;
>  }
>  
> @@ -932,8 +931,8 @@ static long encrypted_read(const struct key *key, char *buffer,
>  		goto out;
>  
>  	ascii_buf = datablob_format(epayload, asciiblob_len);
> -	if (!ascii_buf) {
> -		ret = -ENOMEM;
> +	if (IS_ERR(ascii_buf)) {
> +		ret = PTR_ERR(ascii_buf);
>  		goto out;
>  	}
>  
> -- 
> 2.51.0
> 

No thank you as this does not really fix anything.

BR, Jarkko

^ permalink raw reply

* Re: [PATCH] KEYS: Remove the ad-hoc compilation flag CAAM_DEBUG
From: Jarkko Sakkinen @ 2025-11-09  4:44 UTC (permalink / raw)
  To: Ye Bin
  Cc: a.fatoum, kernel, James.Bottomley, zohar, dhowells, paul, jmorris,
	serge, linux-integrity, keyrings, linux-security-module, yebin10
In-Reply-To: <20251028132254.841715-1-yebin@huaweicloud.com>

On Tue, Oct 28, 2025 at 09:22:54PM +0800, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> Fix the broken design based on Jarkko Sakkinen's suggestions as follows:
> 
> 1. Remove the ad-hoc compilation flag (i.e., CAAM_DEBUG).
> 2. Substitute pr_info calls with pr_debug calls.
> 
> Closes: https://patchwork.kernel.org/project/linux-integrity/patch/20251024061153.61470-1-yebin@huaweicloud.com/
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
>  security/keys/trusted-keys/trusted_caam.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
> index 601943ce0d60..c903ee7328ca 100644
> --- a/security/keys/trusted-keys/trusted_caam.c
> +++ b/security/keys/trusted-keys/trusted_caam.c
> @@ -28,16 +28,10 @@ static const match_table_t key_tokens = {
>  	{opt_err, NULL}
>  };
>  
> -#ifdef CAAM_DEBUG
>  static inline void dump_options(const struct caam_pkey_info *pkey_info)
>  {
> -	pr_info("key encryption algo %d\n", pkey_info->key_enc_algo);
> +	pr_debug("key encryption algo %d\n", pkey_info->key_enc_algo);
>  }
> -#else
> -static inline void dump_options(const struct caam_pkey_info *pkey_info)
> -{
> -}
> -#endif
>  
>  static int get_pkey_options(char *c,
>  			    struct caam_pkey_info *pkey_info)
> -- 
> 2.34.1
> 


Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko

^ permalink raw reply

* Re: Module signing and post-quantum crypto public key algorithms
From: David Howells @ 2025-11-08  7:46 UTC (permalink / raw)
  To: Elliott, Robert (Servers)
  Cc: dhowells, Simo Sorce, James Bottomley, Ignat Korchagin,
	Herbert Xu, Stephan Mueller, torvalds@linux-foundation.org,
	Paul Moore, Lukas Wunner, Clemens Lang, David Bohannon,
	Roberto Sassu, keyrings@vger.kernel.org,
	linux-crypto@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <IA4PR84MB4011FE5ABA934DEF08F1A263ABC3A@IA4PR84MB4011.NAMPRD84.PROD.OUTLOOK.COM>

Elliott, Robert (Servers) <elliott@hpe.com> wrote:

> The traditional signature would use whatever algorithm is used today.
> Legacy verifiers would only check that.

Would there be any legacy verifiers?  Kernel modules are generally tied to the
kernel version for which they were compiled.  Granted there are things like
the wifi regulatory stuff that are also signed.

But note also, PKCS#7 supports multiple independent signatures in a single
object.  The kernel will handle this already.  At least one signature must be
verifiable and none must be blacklisted.

I assume that the main aim of using a composite algorithm is to share the
result of the content hash - but in this case only the authenticatedAttributes
are going to be hashed for the signature, and that's relatively small.

David


^ permalink raw reply

* RE: Module signing and post-quantum crypto public key algorithms
From: Elliott, Robert (Servers) @ 2025-11-07 23:10 UTC (permalink / raw)
  To: Simo Sorce, James Bottomley, Ignat Korchagin, David Howells
  Cc: Herbert Xu, Stephan Mueller, torvalds@linux-foundation.org,
	Paul Moore, Lukas Wunner, Clemens Lang, David Bohannon,
	Roberto Sassu, keyrings@vger.kernel.org,
	linux-crypto@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <3d650cc9ff07462e5c55cc3d9c0da72a3f2c5df2.camel@redhat.com>



> -----Original Message-----
> From: Simo Sorce <simo@redhat.com>
> Sent: Monday, June 16, 2025 12:27 PM
> Subject: Re: Module signing and post-quantum crypto public key
> algorithms
> 
...
> Of course we can decide to hedge *all bets* and move to a composed
> signature (both a classic and a PQ one), in which case I would suggest
> looking into signatures that use ML-DSA-87 + Ed448 or ML-DSA-87 + P-521
> ,ideally disjoint, with a kernel policy that can decide which (or both)
> needs to be valid/checked so that the policy can be changed quickly via
> configuration if any of the signature is broken.
> 
> This will allow for fears of Lattice not being vetted enough to be
> managed as well as increasing the strength of the classic option, while
> maintaining key and signature sizes manageable.

I like the ML-DSA-87 + Ed448 combination.

Consider signing with two signatures: traditional and composite.

The traditional signature would use whatever algorithm is used today.
Legacy verifiers would only check that.

The composite signature would use:
    ML-DSA-87 + Ed448 (+ SHAKE256 if using the CMS composite construction)

New verifiers would only check the composite signature (which requires
checking both of its parts).

That composite would be
* FIPS compliant (all the algorithms are)
* CNSA compliant (ML-DSA-87 makes it so; the rest is just noise)
* compliant with any European requirements or preferences for using hybrids
* compliant with any requirements or preferences for high security strengths
* even with two algorithms, faster than the traditional choices for signing,
  and faster than ECDSA but slower than RSASSA for verification. By bringing
  in Ed448, it might be viewed as an improvement even by PQC skeptics.

It'd be nice if signed kernel modules and packages used the same 
algorithms. Both of these define ML-DSA-87 + Ed448 composites:
* draft-ietf-lamps-pq-composite-sigs
  * id-MLDSA87-Ed448-SHAKE256 with OID: 1.3.6.1.5.5.7.6.51
* draft-ietf-openpgp-pqc
  * ML-DSA-87+Ed448 as public key algorithm ID 31



^ permalink raw reply

* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Mimi Zohar @ 2025-11-07 19:28 UTC (permalink / raw)
  To: Coiby Xu
  Cc: Paul Moore, linux-integrity, linux-security-module, Karel Srot,
	James Morris, Serge E. Hallyn, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
	Eric Snowberg, open list, open list:MODULE SUPPORT
In-Reply-To: <b9eb78105115a00731b3677a5f3a39d5dde4d2ec.camel@linux.ibm.com>

On Thu, 2025-11-06 at 17:15 -0500, Mimi Zohar wrote:
> On Thu, 2025-11-06 at 21:29 +0800, Coiby Xu wrote:
> > On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
> > > On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
> > [...]
> > > 
> > > Hi Coiby,
> > > 
> > > Based on the conversation with Paul, there is no reason to remove the existing
> > > security_kernel_post_read_file() call.
> > > 
> > > The changes are similar to the 2nd link, but a bit different.
> > > - Define a single enumeration named READING_MODULE_COMPRESSED.
> > > 
> > > - In module/main.c add a new security_kernel_post_read_file() call immediately
> > > after decompressing the kernel module.  Like a previous version of this patch,
> > > call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
> > > based on MODULE_INIT_COMPRESSED_FILE.
> > > 
> > > - In ima_post_read_file() defer verifying the signature when the enumeration is
> > > READING_MODULE_COMPRESSED.  (No need for a new function ima_read_kernel_module.)
> > 
> > Hi Mimi,
> > 
> > Thanks for summarizing your conversation with Paul! I can confirm Paul's
> > approach works
> > https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
> > 
> > While testing the patch today, I realized there is another
> > issue/challenge introduced by in-kernel module decompression. IMA
> > appraisal is to verify the digest of compressed kernel module but
> > currently the passed buffer is uncompressed module. When IMA uses
> > uncompressed module data to calculate the digest, xattr signature
> > verification will fail. If we always make IMA read the original kernel
> > module data again to calculate the digest, does it look like a
> > quick-and-dirty fix? If we can assume people won't load kernel module so
> > often, the performance impact is negligible. Otherwise we may have to
> > introduce a new LSM hook so IMA can access uncompressed and original
> > module data one time.
> 
> ima_collect_measurement() stores the file hash info in the iint and uses that
> information to verify the signature as stored in the security xattr. 
> Decompressing the kernel module shouldn't affect the xattr signature
> verification.

In the case when the compressed kernel module hasn't previously been measured or
appraised before loading the kernel module, we need to "collect" the file data
hash on READING_MODULE_COMPRESSED, but defer appraising/measuring it.

An alternative to your suggestion of re-reading the original kernel module data
to calculate the digest or defining a new hook, would be to define "collect" as
a new "action" and pass the kernel_read_file_id enumeration to
process_measurement().  IMA_COLLECTED already exists.  Only IMA_COLLECT would
need to be defined.  The new collect "action" should be limited to
func=MODULE_CHECK.

The downside of this alternative is that it requires a new collect rule:
collect func=MODULE_CHECK mask=MAY_READ uid=0
appraise func=MODULE_CHECK appraise_type=imasig|modsig

-- 
thanks,

Mimi

^ permalink raw reply

* Re: Module signing and post-quantum crypto public key algorithms
From: Stefan Berger @ 2025-11-07 19:19 UTC (permalink / raw)
  To: David Howells
  Cc: Simo Sorce, James Bottomley, Ignat Korchagin, Herbert Xu,
	Stephan Mueller, torvalds, Paul Moore, Lukas Wunner, Clemens Lang,
	David Bohannon, Roberto Sassu, keyrings, linux-crypto,
	linux-security-module, linux-kernel
In-Reply-To: <61528.1762509829@warthog.procyon.org.uk>



On 11/7/25 5:03 AM, David Howells wrote:
> Stefan Berger <stefanb@linux.ibm.com> wrote:
> 
>> On 6/16/25 1:27 PM, Simo Sorce wrote:
>>> Of course we can decide to hedge *all bets* and move to a composed
>>> signature (both a classic and a PQ one), in which case I would suggest
>>> looking into signatures that use ML-DSA-87 + Ed448 or ML-DSA-87 + P-521
>>> ,ideally disjoint, with a kernel policy that can decide which (or both)
>>> needs to be valid/checked so that the policy can be changed quickly via
>>> configuration if any of the signature is broken.
>>
>> FYI: based on this implementation of ML-DSA-44/65/87
>>
>> https://github.com/IBM/mlca/tree/main/qsc/crystals
> 
> The problem with that is that the Apache-2 licence is incompatible with GPLv2.
> Now, it might be possible to persuade IBM to dual-license their code.

Correct. It was supposed to become GPLv2 + Apache 2.

> 
> David
> 
> 


^ permalink raw reply

* [syzbot] [lsm?] WARNING in put_cred_rcu
From: syzbot @ 2025-11-07 14:05 UTC (permalink / raw)
  To: linux-kernel, linux-security-module, luto, peterz, syzkaller-bugs,
	tglx

Hello,

syzbot found the following issue on:

HEAD commit:    17490bd0527f Add linux-next specific files for 20251104
git tree:       linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=16c09bcd980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=9995c0d2611ab121
dashboard link: https://syzkaller.appspot.com/bug?extid=553c4078ab14e3cf3358
compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1500a532580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=12e0a114580000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/a4d318147846/disk-17490bd0.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/86641a470170/vmlinux-17490bd0.xz
kernel image: https://storage.googleapis.com/syzbot-assets/35c008a540c8/bzImage-17490bd0.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+553c4078ab14e3cf3358@syzkaller.appspotmail.com

------------[ cut here ]------------
WARNING: ./include/linux/ns_common.h:255 at __ns_ref_put include/linux/ns_common.h:255 [inline], CPU#0: syz.1.27/6120
WARNING: ./include/linux/ns_common.h:255 at put_user_ns include/linux/user_namespace.h:189 [inline], CPU#0: syz.1.27/6120
WARNING: ./include/linux/ns_common.h:255 at put_cred_rcu+0x2c5/0x340 kernel/cred.c:61, CPU#0: syz.1.27/6120
Modules linked in:
CPU: 0 UID: 0 PID: 6120 Comm: syz.1.27 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/02/2025
RIP: 0010:__ns_ref_put include/linux/ns_common.h:255 [inline]
RIP: 0010:put_user_ns include/linux/user_namespace.h:189 [inline]
RIP: 0010:put_cred_rcu+0x2c5/0x340 kernel/cred.c:61
Code: 5c 41 5d 41 5e 41 5f 5d e9 b8 41 8d 00 e8 03 77 32 00 4c 89 e7 be 03 00 00 00 e8 d6 19 07 03 e9 b8 fe ff ff e8 ec 76 32 00 90 <0f> 0b 90 eb 9f e8 e1 76 32 00 4c 89 ff be 03 00 00 00 e8 b4 19 07
RSP: 0018:ffffc90000007ba8 EFLAGS: 00010246
RAX: ffffffff818ee5a4 RBX: ffff88801c3762a0 RCX: ffff888027350000
RDX: 0000000000000100 RSI: 0000000000000004 RDI: 0000000000000000
RBP: 0000000000000004 R08: ffff888073d31eab R09: 1ffff1100e7a63d5
R10: dffffc0000000000 R11: ffffed100e7a63d6 R12: dffffc0000000000
R13: ffff88801c376200 R14: ffff888073d31d18 R15: ffff888073d31ea8
FS:  00007fcce9f9d6c0(0000) GS:ffff888125a92000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00005555684fe808 CR3: 000000007ee2c000 CR4: 00000000003526f0
Call Trace:
 <IRQ>
 rcu_do_batch kernel/rcu/tree.c:2605 [inline]
 rcu_core+0xcab/0x1770 kernel/rcu/tree.c:2861
 handle_softirqs+0x286/0x870 kernel/softirq.c:626
 __do_softirq kernel/softirq.c:660 [inline]
 invoke_softirq kernel/softirq.c:496 [inline]
 __irq_exit_rcu+0xca/0x1f0 kernel/softirq.c:727
 irq_exit_rcu+0x9/0x30 kernel/softirq.c:743
 instr_sysvec_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1052 [inline]
 sysvec_apic_timer_interrupt+0xa6/0xc0 arch/x86/kernel/apic/apic.c:1052
 </IRQ>
 <TASK>
 asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:697
RIP: 0010:lock_acquire+0x175/0x360 kernel/locking/lockdep.c:5872
Code: 00 00 00 00 9c 8f 44 24 30 f7 44 24 30 00 02 00 00 0f 85 cd 00 00 00 f7 44 24 08 00 02 00 00 74 01 fb 65 48 8b 05 bb 94 1a 11 <48> 3b 44 24 58 0f 85 f2 00 00 00 48 83 c4 60 5b 41 5c 41 5d 41 5e
RSP: 0018:ffffc90002ec5f58 EFLAGS: 00000206
RAX: c3b7b02cbda44300 RBX: 0000000000000000 RCX: c3b7b02cbda44300
RDX: 0000000000000000 RSI: ffffffff8dc69c71 RDI: ffffffff8be0fc60
RBP: ffffffff817420d5 R08: 0000000000000000 R09: ffffffff817420d5
R10: ffffc90002ec6118 R11: ffffffff81ad6a10 R12: 0000000000000002
R13: ffffffff8e33b520 R14: 0000000000000000 R15: 0000000000000246
 rcu_lock_acquire include/linux/rcupdate.h:331 [inline]
 rcu_read_lock include/linux/rcupdate.h:867 [inline]
 class_rcu_constructor include/linux/rcupdate.h:1195 [inline]
 unwind_next_frame+0xc2/0x2390 arch/x86/kernel/unwind_orc.c:479
 arch_stack_walk+0x11c/0x150 arch/x86/kernel/stacktrace.c:25
 stack_trace_save+0x9c/0xe0 kernel/stacktrace.c:122
 save_stack+0xf5/0x1f0 mm/page_owner.c:165
 __set_page_owner+0x8d/0x4c0 mm/page_owner.c:341
 set_page_owner include/linux/page_owner.h:32 [inline]
 post_alloc_hook+0x240/0x2a0 mm/page_alloc.c:1851
 prep_new_page mm/page_alloc.c:1859 [inline]
 get_page_from_freelist+0x2365/0x2440 mm/page_alloc.c:3920
 __alloc_frozen_pages_noprof+0x181/0x370 mm/page_alloc.c:5214
 alloc_pages_mpol+0x232/0x4a0 mm/mempolicy.c:2479
 folio_alloc_mpol_noprof+0x39/0x70 mm/mempolicy.c:2498
 shmem_alloc_folio mm/shmem.c:1900 [inline]
 shmem_alloc_and_add_folio+0x423/0xf40 mm/shmem.c:1942
 shmem_get_folio_gfp+0x59d/0x1660 mm/shmem.c:2565
 shmem_get_folio mm/shmem.c:2671 [inline]
 shmem_write_begin+0xf7/0x2b0 mm/shmem.c:3320
 generic_perform_write+0x2c5/0x900 mm/filemap.c:4313
 shmem_file_write_iter+0xf8/0x120 mm/shmem.c:3495
 __kernel_write_iter+0x428/0x910 fs/read_write.c:619
 dump_emit_page fs/coredump.c:1298 [inline]
 dump_user_range+0x8a0/0xc90 fs/coredump.c:1372
 elf_core_dump+0x3369/0x3960 fs/binfmt_elf.c:2111
 coredump_write+0x116c/0x1900 fs/coredump.c:1049
 vfs_coredump+0x1db5/0x2a60 fs/coredump.c:1170
 get_signal+0x1108/0x1340 kernel/signal.c:3019
 arch_do_signal_or_restart+0xa0/0x790 arch/x86/kernel/signal.c:337
 exit_to_user_mode_loop kernel/entry/common.c:40 [inline]
 exit_to_user_mode_prepare include/linux/irq-entry-common.h:225 [inline]
 irqentry_exit_to_user_mode+0x7e/0x170 kernel/entry/common.c:73
 exc_page_fault+0xab/0x100 arch/x86/mm/fault.c:1535
 asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:618
RIP: 0033:0x7fcce918f6d1
Code: 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 48 3d 01 f0 ff ff 73 01 <c3> 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f
RSP: 002b:0000000000000010 EFLAGS: 00010217
RAX: 0000000000000000 RBX: 00007fcce93e5fa0 RCX: 00007fcce918f6c9
RDX: 0000000000000000 RSI: 0000000000000010 RDI: 498144ee5f62e049
RBP: 00007fcce9211f91 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
R13: 00007fcce93e6038 R14: 00007fcce93e5fa0 R15: 00007ffd4dc9bfc8
 </TASK>
----------------
Code disassembly (best guess):
   0:	00 00                	add    %al,(%rax)
   2:	00 00                	add    %al,(%rax)
   4:	9c                   	pushf
   5:	8f 44 24 30          	pop    0x30(%rsp)
   9:	f7 44 24 30 00 02 00 	testl  $0x200,0x30(%rsp)
  10:	00
  11:	0f 85 cd 00 00 00    	jne    0xe4
  17:	f7 44 24 08 00 02 00 	testl  $0x200,0x8(%rsp)
  1e:	00
  1f:	74 01                	je     0x22
  21:	fb                   	sti
  22:	65 48 8b 05 bb 94 1a 	mov    %gs:0x111a94bb(%rip),%rax        # 0x111a94e5
  29:	11
* 2a:	48 3b 44 24 58       	cmp    0x58(%rsp),%rax <-- trapping instruction
  2f:	0f 85 f2 00 00 00    	jne    0x127
  35:	48 83 c4 60          	add    $0x60,%rsp
  39:	5b                   	pop    %rbx
  3a:	41 5c                	pop    %r12
  3c:	41 5d                	pop    %r13
  3e:	41 5e                	pop    %r14


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

^ permalink raw reply

* Re: [Patch V1] ima: avoid duplicate policy rules insertions
From: Roberto Sassu @ 2025-11-07 11:56 UTC (permalink / raw)
  To: Lennart Poettering
  Cc: Tahera Fahimi, zohar, roberto.sassu, dmitry.kasatkin,
	eric.snowberg, paul, jmorris, serge, linux-integrity,
	linux-security-module, linux-kernel, code
In-Reply-To: <aQ3QV03_PtB4qg32@gardel-login>

On Fri, 2025-11-07 at 11:56 +0100, Lennart Poettering wrote:
> On Fr, 07.11.25 10:44, Roberto Sassu (roberto.sassu@huaweicloud.com) wrote:
> 
> > On Thu, 2025-11-06 at 18:14 +0000, Tahera Fahimi wrote:
> > > Prevent redundant IMA policy rules by checking for duplicates before insertion. This ensures that
> > > rules are not re-added when userspace is restarted (using systemd-soft-reboot) without a full system
> > > reboot. ima_rule_exists() detects duplicates in both temporary and active rule lists.
> > 
> > + Lennart
> > 
> > Hi Tahera
> > 
> > thanks for the patch!
> > 
> > Wouldn't be better to enhance systemd-soft-reboot to not send the same
> > IMA policy again?
> 
> the soft-reboot logic doesn't load the IMA policy. It's just that
> soft-reboot means we reexec PID1: the old pid1 gets replaced by the
> new one. And that new PID1 then initializes as it usually would, and
> loads security policies again. It currently has support for selinux
> policies, ima, ipe, smack.
> 
> These policies are supposed to *replace* whatever was loaded
> before. Looking at our IMA logic, this doesn't happen right now
> though, it just adds stuff:

From a functional perspective. As far as integrity is concerned, you
would probably agree that just replacing PID 1 does not mean resetting
the integrity of the system to a known state (all the other processes
are still running). Due to that, I think the concept of soft-reset
should not be applied to the kernel.

> https://github.com/systemd/systemd/blob/main/src/core/ima-setup.c
> 
> Is there a way to replace the old IMA policy with the new, with the
> current IMA userspace interface? If so, we should probably make use of

No, only the IMA boot policies specified in the kernel command line can
be reset (only once, and not completely, secure boot rules still
persist despite user space loads a new policy). New rules are append-
only.

> that in systemd, and replace the policy that way. Or in other words:
> under the assumption that one can flush out the old IMA policy and
> replace it with a new one, I think this should be fixed in systemd,
> not the kernel. (of there's no api for flushing out the old
> policy/replacing it with the new, then of course we need something
> like that in the kernel first).

Assuming that technically it is feasible to flush the old IMA policy
(except for the permanent secure boot rules). What it would be the
additional value of changing the policy on the fly on the same system
as before, but with a different PID 1?

Regarding the duplicate IMA policy load, I guess you could probably
store the digest of the currently loaded policy in the systemd state
and not doing it again after soft-reboot, if the digest matches.

Roberto

> My understanding of IMA is kinda limited though. I just know what we
> do in our codebase.
> 
> Lennart


^ permalink raw reply

* Re: [Patch V1] ima: avoid duplicate policy rules insertions
From: Lennart Poettering @ 2025-11-07 10:56 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: Tahera Fahimi, zohar, roberto.sassu, dmitry.kasatkin,
	eric.snowberg, paul, jmorris, serge, linux-integrity,
	linux-security-module, linux-kernel, code
In-Reply-To: <1cc67c25a141aef8982840898a6e7397cbdf10d9.camel@huaweicloud.com>

On Fr, 07.11.25 10:44, Roberto Sassu (roberto.sassu@huaweicloud.com) wrote:

> On Thu, 2025-11-06 at 18:14 +0000, Tahera Fahimi wrote:
> > Prevent redundant IMA policy rules by checking for duplicates before insertion. This ensures that
> > rules are not re-added when userspace is restarted (using systemd-soft-reboot) without a full system
> > reboot. ima_rule_exists() detects duplicates in both temporary and active rule lists.
>
> + Lennart
>
> Hi Tahera
>
> thanks for the patch!
>
> Wouldn't be better to enhance systemd-soft-reboot to not send the same
> IMA policy again?

the soft-reboot logic doesn't load the IMA policy. It's just that
soft-reboot means we reexec PID1: the old pid1 gets replaced by the
new one. And that new PID1 then initializes as it usually would, and
loads security policies again. It currently has support for selinux
policies, ima, ipe, smack.

These policies are supposed to *replace* whatever was loaded
before. Looking at our IMA logic, this doesn't happen right now
though, it just adds stuff:

https://github.com/systemd/systemd/blob/main/src/core/ima-setup.c

Is there a way to replace the old IMA policy with the new, with the
current IMA userspace interface? If so, we should probably make use of
that in systemd, and replace the policy that way. Or in other words:
under the assumption that one can flush out the old IMA policy and
replace it with a new one, I think this should be fixed in systemd,
not the kernel. (of there's no api for flushing out the old
policy/replacing it with the new, then of course we need something
like that in the kernel first).

My understanding of IMA is kinda limited though. I just know what we
do in our codebase.

Lennart

^ permalink raw reply

* Re: Module signing and post-quantum crypto public key algorithms
From: Stephan Mueller @ 2025-11-07 10:23 UTC (permalink / raw)
  To: Stefan Berger, David Howells
  Cc: dhowells, Simo Sorce, James Bottomley, Ignat Korchagin,
	Herbert Xu, torvalds, Paul Moore, Lukas Wunner, Clemens Lang,
	David Bohannon, Roberto Sassu, keyrings, linux-crypto,
	linux-security-module, linux-kernel
In-Reply-To: <61528.1762509829@warthog.procyon.org.uk>

Am Freitag, 7. November 2025, 11:03:49 Mitteleuropäische Normalzeit schrieb 
David Howells:

Hi David,

> Stefan Berger <stefanb@linux.ibm.com> wrote:
> > On 6/16/25 1:27 PM, Simo Sorce wrote:
> > > Of course we can decide to hedge *all bets* and move to a composed
> > > signature (both a classic and a PQ one), in which case I would suggest
> > > looking into signatures that use ML-DSA-87 + Ed448 or ML-DSA-87 + P-521
> > > ,ideally disjoint, with a kernel policy that can decide which (or both)
> > > needs to be valid/checked so that the policy can be changed quickly via
> > > configuration if any of the signature is broken.
> > 
> > FYI: based on this implementation of ML-DSA-44/65/87
> > 
> > https://github.com/IBM/mlca/tree/main/qsc/crystals
> 
> The problem with that is that the Apache-2 licence is incompatible with
> GPLv2. Now, it might be possible to persuade IBM to dual-license their
> code.

The code used as a basis for the current approach (leancrypto.org) offers 
hybrids with ED25519 and ED448) including the X.509/PKCS7 support.

However, please note that the X.509 specification for storing hybrid public 
keys is not yet completed and there is still some work on the draft IETF 
standard [1].

Side note: the leancrypto code base uses the Linux kernel X.509/PKCS7 parser 
code where the relevant handler functions (see [2]) could be relatively 
quickly transplanted into the kernel if needed.

[1] https://lamps-wg.github.io/draft-composite-sigs/draft-ietf-lamps-pq-composite-sigs.html

[2] https://github.com/smuellerDD/leancrypto/blob/master/asn1/src/
x509_cert_parser.c

Ciao
Stephan



^ permalink raw reply

* Re: Module signing and post-quantum crypto public key algorithms
From: David Howells @ 2025-11-07 10:03 UTC (permalink / raw)
  To: Stefan Berger
  Cc: dhowells, Simo Sorce, James Bottomley, Ignat Korchagin,
	Herbert Xu, Stephan Mueller, torvalds, Paul Moore, Lukas Wunner,
	Clemens Lang, David Bohannon, Roberto Sassu, keyrings,
	linux-crypto, linux-security-module, linux-kernel
In-Reply-To: <53e81761-47e1-400e-933d-0a53018c9cab@linux.ibm.com>

Stefan Berger <stefanb@linux.ibm.com> wrote:

> On 6/16/25 1:27 PM, Simo Sorce wrote:
> > Of course we can decide to hedge *all bets* and move to a composed
> > signature (both a classic and a PQ one), in which case I would suggest
> > looking into signatures that use ML-DSA-87 + Ed448 or ML-DSA-87 + P-521
> > ,ideally disjoint, with a kernel policy that can decide which (or both)
> > needs to be valid/checked so that the policy can be changed quickly via
> > configuration if any of the signature is broken.
> 
> FYI: based on this implementation of ML-DSA-44/65/87
> 
> https://github.com/IBM/mlca/tree/main/qsc/crystals

The problem with that is that the Apache-2 licence is incompatible with GPLv2.
Now, it might be possible to persuade IBM to dual-license their code.

David


^ permalink raw reply

* Re: [Patch V1] ima: avoid duplicate policy rules insertions
From: Roberto Sassu @ 2025-11-07  9:44 UTC (permalink / raw)
  To: Tahera Fahimi, zohar, roberto.sassu, dmitry.kasatkin,
	eric.snowberg, paul, jmorris, serge, linux-integrity,
	linux-security-module, linux-kernel, code
  Cc: Lennart Poettering
In-Reply-To: <20251106181404.3429710-1-taherafahimi@linux.microsoft.com>

On Thu, 2025-11-06 at 18:14 +0000, Tahera Fahimi wrote:
> Prevent redundant IMA policy rules by checking for duplicates before insertion. This ensures that
> rules are not re-added when userspace is restarted (using systemd-soft-reboot) without a full system
> reboot. ima_rule_exists() detects duplicates in both temporary and active rule lists.

+ Lennart

Hi Tahera

thanks for the patch!


Wouldn't be better to enhance systemd-soft-reboot to not send the same
IMA policy again?

Thanks

Roberto

> Signed-off-by: Tahera Fahimi <taherafahimi@linux.microsoft.com>
> ---
>  security/integrity/ima/ima_policy.c | 157 +++++++++++++++++++++++++++-
>  1 file changed, 156 insertions(+), 1 deletion(-)
> 
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 164d62832f8ec..3dd902101dbda 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -1953,6 +1953,153 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  	return result;
>  }
>  
> +static bool template_has_field(const char *field_id, const struct ima_template_desc *template2)
> +{
> +	int j;
> +
> +	for (int j = 0; j < template2->num_fields; j++)
> +		if (strcmp(field_id, template2->fields[j]->field_id) == 0)
> +			return true;
> +
> +	return false;
> +}
> +
> +static bool keyring_has_item(const char *item, const struct ima_rule_opt_list *keyrings)
> +{
> +	int j;
> +
> +	for (j = 0; j < keyrings->count; j++) {
> +		if (strcmp(item, keyrings->items[j]) == 0)
> +			return true;
> +	}
> +	return false;
> +}
> +
> +static bool labels_has_item(const char *item, const struct ima_rule_opt_list *labels)
> +{
> +	int j;
> +
> +	for (j = 0; j < labels->count; j++) {
> +		if (strcmp(item, labels->items[j]) == 0)
> +			return true;
> +	}
> +	return false;
> +}
> +
> +static bool ima_rules_equal(const struct ima_rule_entry *rule1, const struct ima_rule_entry *rule2)
> +{
> +	int i;
> +
> +	if (rule1->flags != rule2->flags)
> +		return false;
> +
> +	if (rule1->action != rule2->action)
> +		return false;
> +
> +	if (((rule1->flags & IMA_FUNC) && rule1->func != rule2->func) ||
> +	    ((rule1->flags & (IMA_MASK | IMA_INMASK)) && rule1->mask != rule2->mask) ||
> +	    ((rule1->flags & IMA_FSMAGIC) && rule1->fsmagic != rule2->fsmagic) ||
> +	    ((rule1->flags & IMA_FSUUID) && !uuid_equal(&rule1->fsuuid, &rule2->fsuuid)) ||
> +	    ((rule1->flags & IMA_UID) && !uid_eq(rule1->uid, rule2->uid)) ||
> +	    ((rule1->flags & IMA_GID) && !gid_eq(rule1->gid, rule2->gid)) ||
> +	    ((rule1->flags & IMA_FOWNER) && !uid_eq(rule1->fowner, rule2->fowner)) ||
> +	    ((rule1->flags & IMA_FGROUP) && !gid_eq(rule1->fgroup, rule2->fgroup)) ||
> +	    ((rule1->flags & IMA_FSNAME) && (strcmp(rule1->fsname, rule2->fsname) != 0)) ||
> +	    ((rule1->flags & IMA_PCR) && rule1->pcr != rule2->pcr) ||
> +	    ((rule1->flags & IMA_VALIDATE_ALGOS) &&
> +	      rule1->allowed_algos != rule2->allowed_algos) ||
> +	    ((rule1->flags & IMA_EUID) && !uid_eq(rule1->uid, rule2->uid)) ||
> +	    ((rule1->flags & IMA_EGID) && !gid_eq(rule1->gid, rule2->gid)))
> +		return false;
> +
> +	if (!rule1->template && !rule2->template) {
> +		;
> +	} else if (!rule1->template || !rule2->template) {
> +		return false;
> +	} else if (rule1->template->num_fields != rule2->template->num_fields) {
> +		return false;
> +	} else if (rule1->template->num_fields != 0) {
> +		for (i = 0; i < rule1->template->num_fields; i++) {
> +			if (!template_has_field(rule1->template->fields[i]->field_id,
> +						rule2->template))
> +				return false;
> +		}
> +	}
> +
> +	if (rule1->flags & IMA_KEYRINGS) {
> +		if (!rule1->keyrings && !rule2->keyrings) {
> +			;
> +		} else if (!rule1->keyrings || !rule2->keyrings) {
> +			return false;
> +		} else if (rule1->keyrings->count != rule2->keyrings->count) {
> +			return false;
> +		} else if (rule1->keyrings->count != 0) {
> +			for (i = 0; i < rule1->keyrings->count; i++) {
> +				if (!keyring_has_item(rule1->keyrings->items[i], rule2->keyrings))
> +					return false;
> +			}
> +		}
> +	}
> +
> +	if (rule1->flags & IMA_LABEL) {
> +		if (!rule1->label && !rule2->label) {
> +			;
> +		} else if (!rule1->label || !rule2->label) {
> +			return false;
> +		} else if (rule1->label->count != rule2->label->count) {
> +			return false;
> +		} else if (rule1->label->count != 0) {
> +			for (i = 0; i < rule1->label->count; i++) {
> +				if (!labels_has_item(rule1->label->items[i], rule2->label))
> +					return false;
> +			}
> +		}
> +	}
> +
> +	for (i = 0; i < MAX_LSM_RULES; i++) {
> +		if (!rule1->lsm[i].rule && !rule2->lsm[i].rule)
> +			continue;
> +
> +		if (!rule1->lsm[i].rule || !rule2->lsm[i].rule)
> +			return false;
> +
> +		if (strcmp(rule1->lsm[i].args_p, rule2->lsm[i].args_p) != 0)
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
> +/**
> + * ima_rule_exists - check if a rule already exists in the policy
> + *
> + * Checking both the active policy and the temporary rules list.
> + */
> +static bool ima_rule_exists(struct ima_rule_entry *new_rule)
> +{
> +	struct ima_rule_entry *entry;
> +	struct list_head *ima_rules_tmp;
> +
> +	if (!list_empty(&ima_temp_rules)) {
> +		list_for_each_entry(entry, &ima_temp_rules, list) {
> +			if (ima_rules_equal(entry, new_rule))
> +				return true;
> +		}
> +	}
> +
> +	rcu_read_lock();
> +	ima_rules_tmp = rcu_dereference(ima_rules);
> +	list_for_each_entry_rcu(entry, ima_rules_tmp, list) {
> +		if (ima_rules_equal(entry, new_rule)) {
> +			rcu_read_unlock();
> +			return true;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return false;
> +}
> +
>  /**
>   * ima_parse_add_rule - add a rule to ima_policy_rules
>   * @rule: ima measurement policy rule
> @@ -1993,7 +2140,15 @@ ssize_t ima_parse_add_rule(char *rule)
>  		return result;
>  	}
>  
> -	list_add_tail(&entry->list, &ima_temp_rules);
> +	if (!ima_rule_exists(entry)) {
> +		list_add_tail(&entry->list, &ima_temp_rules);
> +	} else {
> +		result = -EEXIST;
> +		ima_free_rule(entry);
> +		integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
> +				    NULL, op, "duplicate-policy", result,
> +				    audit_info);
> +	}
>  
>  	return len;
>  }


^ permalink raw reply

* [syzbot] [fs?] WARNING in destroy_super_work
From: syzbot @ 2025-11-07  7:31 UTC (permalink / raw)
  To: Liam.Howlett, akpm, anna-maria, bpf, brauner, bsegall, cgroups,
	david, dietmar.eggemann, frederic, hannes, jack, jsavitz,
	juri.lelli, kees, linux-fsdevel, linux-kernel, linux-mm,
	linux-security-module, lorenzo.stoakes, mgorman, mhocko, mingo,
	mjguzik, mkoutny, oleg, paul, peterz, rostedt, rppt, sergeh,
	surenb, syzkaller-bugs, tglx, tj, vbabka, vincent.guittot, viro,
	vschneid

Hello,

syzbot found the following issue on:

HEAD commit:    982312090977 Add linux-next specific files for 20251103
git tree:       linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=17b2932f980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=43cc0e31558cb527
dashboard link: https://syzkaller.appspot.com/bug?extid=1957b26299cf3ff7890c
compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1347817c580000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/40058f8a830c/disk-98231209.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/1d7f42e8639f/vmlinux-98231209.xz
kernel image: https://storage.googleapis.com/syzbot-assets/d8bb0284f393/bzImage-98231209.xz

The issue was bisected to:

commit 3c9820d5c64aeaadea7ffe3a6bb99d019a5ff46a
Author: Christian Brauner <brauner@kernel.org>
Date:   Wed Oct 29 12:20:24 2025 +0000

    ns: add active reference count

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=101e9bcd980000
final oops:     https://syzkaller.appspot.com/x/report.txt?x=121e9bcd980000
console output: https://syzkaller.appspot.com/x/log.txt?x=141e9bcd980000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+1957b26299cf3ff7890c@syzkaller.appspotmail.com
Fixes: 3c9820d5c64a ("ns: add active reference count")

------------[ cut here ]------------
WARNING: ./include/linux/ns_common.h:229 at __ns_ref_put include/linux/ns_common.h:229 [inline], CPU#0: kworker/0:6/6108
WARNING: ./include/linux/ns_common.h:229 at put_user_ns include/linux/user_namespace.h:189 [inline], CPU#0: kworker/0:6/6108
WARNING: ./include/linux/ns_common.h:229 at destroy_super_work+0x15c/0x1a0 fs/super.c:280, CPU#0: kworker/0:6/6108
Modules linked in:
CPU: 0 UID: 0 PID: 6108 Comm: kworker/0:6 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/02/2025
Workqueue: events destroy_super_work
RIP: 0010:__ns_ref_put include/linux/ns_common.h:229 [inline]
RIP: 0010:put_user_ns include/linux/user_namespace.h:189 [inline]
RIP: 0010:destroy_super_work+0x15c/0x1a0 fs/super.c:280
Code: 90 63 ff 48 81 c3 a8 fc ff ff 48 89 df e8 ec 90 63 ff 4c 89 f7 5b 41 5c 41 5d 41 5e 41 5f 5d e9 8a 91 e1 ff e8 45 df 86 ff 90 <0f> 0b 90 e9 6d ff ff ff e8 37 df 86 ff 4c 89 e7 be 03 00 00 00 e8
RSP: 0018:ffffc900030d7a48 EFLAGS: 00010293
RAX: ffffffff823a294b RBX: ffff88805639c898 RCX: ffff88802ab59e80
RDX: 0000000000000000 RSI: 0000000000000004 RDI: 0000000000000000
RBP: 0000000000000004 R08: ffff88807477565b R09: 1ffff1100e8eeacb
R10: dffffc0000000000 R11: ffffed100e8eeacc R12: ffff888074775658
R13: dffffc0000000000 R14: ffff88805639c000 R15: ffff8880747754c8
FS:  0000000000000000(0000) GS:ffff888125eda000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000c008044000 CR3: 0000000077ad4000 CR4: 00000000003526f0
Call Trace:
 <TASK>
 process_one_work+0x94a/0x15d0 kernel/workqueue.c:3267
 process_scheduled_works kernel/workqueue.c:3350 [inline]
 worker_thread+0x9b0/0xee0 kernel/workqueue.c:3431
 kthread+0x711/0x8a0 kernel/kthread.c:463
 ret_from_fork+0x52d/0xa70 arch/x86/kernel/process.c:158
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
 </TASK>


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

^ permalink raw reply

* [syzbot] [lsm?] memory leak in prepare_creds (5)
From: syzbot @ 2025-11-07  7:29 UTC (permalink / raw)
  To: linux-kernel, linux-security-module, paul, sergeh, syzkaller-bugs

Hello,

syzbot found the following issue on:

HEAD commit:    c2c2ccfd4ba7 Merge tag 'net-6.18-rc5' of git://git.kernel...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13d6ca92580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=cb128cd5cb439809
dashboard link: https://syzkaller.appspot.com/bug?extid=099461f8558eb0a1f4f3
compiler:       gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11ac7012580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=12aa10b4580000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/b0451ba3fe41/disk-c2c2ccfd.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/d3e8c67119ab/vmlinux-c2c2ccfd.xz
kernel image: https://storage.googleapis.com/syzbot-assets/1d8e176e5054/bzImage-c2c2ccfd.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+099461f8558eb0a1f4f3@syzkaller.appspotmail.com

2025/11/07 06:07:22 executed programs: 5
BUG: memory leak
unreferenced object 0xffff888101ad8900 (size 184):
  comm "syz-executor", pid 5986, jiffies 4294942797
  hex dump (first 32 bytes):
    01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace (crc 45b79b1):
    kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
    slab_post_alloc_hook mm/slub.c:4975 [inline]
    slab_alloc_node mm/slub.c:5280 [inline]
    kmem_cache_alloc_noprof+0x397/0x5a0 mm/slub.c:5287
    prepare_creds+0x22/0x4f0 kernel/cred.c:212
    copy_creds+0x44/0x290 kernel/cred.c:312
    copy_process+0x706/0x27d0 kernel/fork.c:2046
    kernel_clone+0x119/0x6c0 kernel/fork.c:2609
    __do_sys_clone+0x7b/0xb0 kernel/fork.c:2750
    do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
    do_syscall_64+0xa4/0xfa0 arch/x86/entry/syscall_64.c:94
    entry_SYSCALL_64_after_hwframe+0x77/0x7f

BUG: memory leak
unreferenced object 0xffff88810096ace0 (size 32):
  comm "syz-executor", pid 5986, jiffies 4294942797
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    f8 f2 85 00 81 88 ff ff 00 00 00 00 00 00 00 00  ................
  backtrace (crc 894df7a1):
    kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
    slab_post_alloc_hook mm/slub.c:4975 [inline]
    slab_alloc_node mm/slub.c:5280 [inline]
    __do_kmalloc_node mm/slub.c:5641 [inline]
    __kmalloc_noprof+0x3e3/0x6b0 mm/slub.c:5654
    kmalloc_noprof include/linux/slab.h:961 [inline]
    kzalloc_noprof include/linux/slab.h:1094 [inline]
    lsm_blob_alloc+0x4d/0x70 security/security.c:690
    lsm_cred_alloc security/security.c:707 [inline]
    security_prepare_creds+0x30/0x270 security/security.c:3310
    prepare_creds+0x346/0x4f0 kernel/cred.c:242
    copy_creds+0x44/0x290 kernel/cred.c:312
    copy_process+0x706/0x27d0 kernel/fork.c:2046
    kernel_clone+0x119/0x6c0 kernel/fork.c:2609
    __do_sys_clone+0x7b/0xb0 kernel/fork.c:2750
    do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
    do_syscall_64+0xa4/0xfa0 arch/x86/entry/syscall_64.c:94
    entry_SYSCALL_64_after_hwframe+0x77/0x7f

BUG: memory leak
unreferenced object 0xffff888128f3ca00 (size 1264):
  comm "kworker/1:1", pid 42, jiffies 4294942797
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    28 00 0b 40 00 00 00 00 00 00 00 00 00 00 00 00  (..@............
  backtrace (crc fdfa4398):
    kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
    slab_post_alloc_hook mm/slub.c:4975 [inline]
    slab_alloc_node mm/slub.c:5280 [inline]
    kmem_cache_alloc_noprof+0x397/0x5a0 mm/slub.c:5287
    sk_prot_alloc+0x3e/0x1b0 net/core/sock.c:2233
    sk_alloc+0x36/0x360 net/core/sock.c:2295
    __vsock_create.constprop.0+0x38/0x2f0 net/vmw_vsock/af_vsock.c:788
    virtio_transport_recv_listen net/vmw_vsock/virtio_transport_common.c:1527 [inline]
    virtio_transport_recv_pkt+0x7fd/0xf30 net/vmw_vsock/virtio_transport_common.c:1655
    vsock_loopback_work+0xfe/0x140 net/vmw_vsock/vsock_loopback.c:133
    process_one_work+0x26b/0x620 kernel/workqueue.c:3263
    process_scheduled_works kernel/workqueue.c:3346 [inline]
    worker_thread+0x2c4/0x4f0 kernel/workqueue.c:3427
    kthread+0x15b/0x310 kernel/kthread.c:463
    ret_from_fork+0x210/0x240 arch/x86/kernel/process.c:158
    ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245

BUG: memory leak
unreferenced object 0xffff888146540100 (size 32):
  comm "kworker/1:1", pid 42, jiffies 4294942797
  hex dump (first 32 bytes):
    f8 f2 85 00 81 88 ff ff 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace (crc e7cc8a40):
    kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
    slab_post_alloc_hook mm/slub.c:4975 [inline]
    slab_alloc_node mm/slub.c:5280 [inline]
    __do_kmalloc_node mm/slub.c:5641 [inline]
    __kmalloc_noprof+0x3e3/0x6b0 mm/slub.c:5654
    kmalloc_noprof include/linux/slab.h:961 [inline]
    kzalloc_noprof include/linux/slab.h:1094 [inline]
    lsm_blob_alloc+0x4d/0x70 security/security.c:690
    lsm_sock_alloc security/security.c:4922 [inline]
    security_sk_alloc+0x30/0x270 security/security.c:4938
    sk_prot_alloc+0x8f/0x1b0 net/core/sock.c:2242
    sk_alloc+0x36/0x360 net/core/sock.c:2295
    __vsock_create.constprop.0+0x38/0x2f0 net/vmw_vsock/af_vsock.c:788
    virtio_transport_recv_listen net/vmw_vsock/virtio_transport_common.c:1527 [inline]
    virtio_transport_recv_pkt+0x7fd/0xf30 net/vmw_vsock/virtio_transport_common.c:1655
    vsock_loopback_work+0xfe/0x140 net/vmw_vsock/vsock_loopback.c:133
    process_one_work+0x26b/0x620 kernel/workqueue.c:3263
    process_scheduled_works kernel/workqueue.c:3346 [inline]
    worker_thread+0x2c4/0x4f0 kernel/workqueue.c:3427
    kthread+0x15b/0x310 kernel/kthread.c:463
    ret_from_fork+0x210/0x240 arch/x86/kernel/process.c:158
    ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245

BUG: memory leak
unreferenced object 0xffff888109c274e0 (size 96):
  comm "kworker/1:1", pid 42, jiffies 4294942797
  hex dump (first 32 bytes):
    00 ca f3 28 81 88 ff ff 00 00 00 00 00 00 00 00  ...(............
    00 00 00 00 00 00 00 00 00 00 04 00 00 00 00 00  ................
  backtrace (crc 4f48ed20):
    kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
    slab_post_alloc_hook mm/slub.c:4975 [inline]
    slab_alloc_node mm/slub.c:5280 [inline]
    __kmalloc_cache_noprof+0x3a6/0x5b0 mm/slub.c:5758
    kmalloc_noprof include/linux/slab.h:957 [inline]
    kzalloc_noprof include/linux/slab.h:1094 [inline]
    virtio_transport_do_socket_init+0x2b/0xf0 net/vmw_vsock/virtio_transport_common.c:910
    vsock_assign_transport+0x31b/0x3a0 net/vmw_vsock/af_vsock.c:537
    virtio_transport_recv_listen net/vmw_vsock/virtio_transport_common.c:1545 [inline]
    virtio_transport_recv_pkt+0x861/0xf30 net/vmw_vsock/virtio_transport_common.c:1655
    vsock_loopback_work+0xfe/0x140 net/vmw_vsock/vsock_loopback.c:133
    process_one_work+0x26b/0x620 kernel/workqueue.c:3263
    process_scheduled_works kernel/workqueue.c:3346 [inline]
    worker_thread+0x2c4/0x4f0 kernel/workqueue.c:3427
    kthread+0x15b/0x310 kernel/kthread.c:463
    ret_from_fork+0x210/0x240 arch/x86/kernel/process.c:158
    ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245

BUG: memory leak
unreferenced object 0xffff888101a80f00 (size 184):
  comm "syz-executor", pid 5986, jiffies 4294942798
  hex dump (first 32 bytes):
    01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace (crc 51e903bf):
    kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
    slab_post_alloc_hook mm/slub.c:4975 [inline]
    slab_alloc_node mm/slub.c:5280 [inline]
    kmem_cache_alloc_noprof+0x397/0x5a0 mm/slub.c:5287
    prepare_creds+0x22/0x4f0 kernel/cred.c:212
    copy_creds+0x44/0x290 kernel/cred.c:312
    copy_process+0x706/0x27d0 kernel/fork.c:2046
    kernel_clone+0x119/0x6c0 kernel/fork.c:2609
    __do_sys_clone+0x7b/0xb0 kernel/fork.c:2750
    do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
    do_syscall_64+0xa4/0xfa0 arch/x86/entry/syscall_64.c:94
    entry_SYSCALL_64_after_hwframe+0x77/0x7f

connection error: failed to recv *flatrpc.ExecutorMessageRawT: EOF


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

^ permalink raw reply

* Re: [Patch V1] ima: avoid duplicate policy rules insertions
From: kernel test robot @ 2025-11-07  6:54 UTC (permalink / raw)
  To: Tahera Fahimi, zohar, roberto.sassu, dmitry.kasatkin,
	eric.snowberg, paul, jmorris, serge, linux-integrity,
	linux-security-module, linux-kernel, code
  Cc: oe-kbuild-all, Tahera Fahimi
In-Reply-To: <20251106181404.3429710-1-taherafahimi@linux.microsoft.com>

Hi Tahera,

kernel test robot noticed the following build warnings:

[auto build test WARNING on zohar-integrity/next-integrity]
[also build test WARNING on linus/master v6.18-rc4 next-20251107]
[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/Tahera-Fahimi/ima-avoid-duplicate-policy-rules-insertions/20251107-021615
base:   https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
patch link:    https://lore.kernel.org/r/20251106181404.3429710-1-taherafahimi%40linux.microsoft.com
patch subject: [Patch V1] ima: avoid duplicate policy rules insertions
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20251107/202511071406.hU1UdCKh-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251107/202511071406.hU1UdCKh-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/202511071406.hU1UdCKh-lkp@intel.com/

All warnings (new ones prefixed by >>):

   security/integrity/ima/ima_policy.c: In function 'template_has_field':
>> security/integrity/ima/ima_policy.c:1958:13: warning: unused variable 'j' [-Wunused-variable]
    1958 |         int j;
         |             ^
--
>> Warning: security/integrity/ima/ima_policy.c:2078 function parameter 'new_rule' not described in 'ima_rule_exists'


vim +/j +1958 security/integrity/ima/ima_policy.c

  1955	
  1956	static bool template_has_field(const char *field_id, const struct ima_template_desc *template2)
  1957	{
> 1958		int j;
  1959	
  1960		for (int j = 0; j < template2->num_fields; j++)
  1961			if (strcmp(field_id, template2->fields[j]->field_id) == 0)
  1962				return true;
  1963	
  1964		return false;
  1965	}
  1966	
  1967	static bool keyring_has_item(const char *item, const struct ima_rule_opt_list *keyrings)
  1968	{
  1969		int j;
  1970	
  1971		for (j = 0; j < keyrings->count; j++) {
  1972			if (strcmp(item, keyrings->items[j]) == 0)
  1973				return true;
  1974		}
  1975		return false;
  1976	}
  1977	
  1978	static bool labels_has_item(const char *item, const struct ima_rule_opt_list *labels)
  1979	{
  1980		int j;
  1981	
  1982		for (j = 0; j < labels->count; j++) {
  1983			if (strcmp(item, labels->items[j]) == 0)
  1984				return true;
  1985		}
  1986		return false;
  1987	}
  1988	
  1989	static bool ima_rules_equal(const struct ima_rule_entry *rule1, const struct ima_rule_entry *rule2)
  1990	{
  1991		int i;
  1992	
  1993		if (rule1->flags != rule2->flags)
  1994			return false;
  1995	
  1996		if (rule1->action != rule2->action)
  1997			return false;
  1998	
  1999		if (((rule1->flags & IMA_FUNC) && rule1->func != rule2->func) ||
  2000		    ((rule1->flags & (IMA_MASK | IMA_INMASK)) && rule1->mask != rule2->mask) ||
  2001		    ((rule1->flags & IMA_FSMAGIC) && rule1->fsmagic != rule2->fsmagic) ||
  2002		    ((rule1->flags & IMA_FSUUID) && !uuid_equal(&rule1->fsuuid, &rule2->fsuuid)) ||
  2003		    ((rule1->flags & IMA_UID) && !uid_eq(rule1->uid, rule2->uid)) ||
  2004		    ((rule1->flags & IMA_GID) && !gid_eq(rule1->gid, rule2->gid)) ||
  2005		    ((rule1->flags & IMA_FOWNER) && !uid_eq(rule1->fowner, rule2->fowner)) ||
  2006		    ((rule1->flags & IMA_FGROUP) && !gid_eq(rule1->fgroup, rule2->fgroup)) ||
  2007		    ((rule1->flags & IMA_FSNAME) && (strcmp(rule1->fsname, rule2->fsname) != 0)) ||
  2008		    ((rule1->flags & IMA_PCR) && rule1->pcr != rule2->pcr) ||
  2009		    ((rule1->flags & IMA_VALIDATE_ALGOS) &&
  2010		      rule1->allowed_algos != rule2->allowed_algos) ||
  2011		    ((rule1->flags & IMA_EUID) && !uid_eq(rule1->uid, rule2->uid)) ||
  2012		    ((rule1->flags & IMA_EGID) && !gid_eq(rule1->gid, rule2->gid)))
  2013			return false;
  2014	
  2015		if (!rule1->template && !rule2->template) {
  2016			;
  2017		} else if (!rule1->template || !rule2->template) {
  2018			return false;
  2019		} else if (rule1->template->num_fields != rule2->template->num_fields) {
  2020			return false;
  2021		} else if (rule1->template->num_fields != 0) {
  2022			for (i = 0; i < rule1->template->num_fields; i++) {
  2023				if (!template_has_field(rule1->template->fields[i]->field_id,
  2024							rule2->template))
  2025					return false;
  2026			}
  2027		}
  2028	
  2029		if (rule1->flags & IMA_KEYRINGS) {
  2030			if (!rule1->keyrings && !rule2->keyrings) {
  2031				;
  2032			} else if (!rule1->keyrings || !rule2->keyrings) {
  2033				return false;
  2034			} else if (rule1->keyrings->count != rule2->keyrings->count) {
  2035				return false;
  2036			} else if (rule1->keyrings->count != 0) {
  2037				for (i = 0; i < rule1->keyrings->count; i++) {
  2038					if (!keyring_has_item(rule1->keyrings->items[i], rule2->keyrings))
  2039						return false;
  2040				}
  2041			}
  2042		}
  2043	
  2044		if (rule1->flags & IMA_LABEL) {
  2045			if (!rule1->label && !rule2->label) {
  2046				;
  2047			} else if (!rule1->label || !rule2->label) {
  2048				return false;
  2049			} else if (rule1->label->count != rule2->label->count) {
  2050				return false;
  2051			} else if (rule1->label->count != 0) {
  2052				for (i = 0; i < rule1->label->count; i++) {
  2053					if (!labels_has_item(rule1->label->items[i], rule2->label))
  2054						return false;
  2055				}
  2056			}
  2057		}
  2058	
  2059		for (i = 0; i < MAX_LSM_RULES; i++) {
  2060			if (!rule1->lsm[i].rule && !rule2->lsm[i].rule)
  2061				continue;
  2062	
  2063			if (!rule1->lsm[i].rule || !rule2->lsm[i].rule)
  2064				return false;
  2065	
  2066			if (strcmp(rule1->lsm[i].args_p, rule2->lsm[i].args_p) != 0)
  2067				return false;
  2068		}
  2069	
  2070		return true;
  2071	}
  2072	
  2073	/**
  2074	 * ima_rule_exists - check if a rule already exists in the policy
  2075	 *
  2076	 * Checking both the active policy and the temporary rules list.
  2077	 */
> 2078	static bool ima_rule_exists(struct ima_rule_entry *new_rule)
  2079	{
  2080		struct ima_rule_entry *entry;
  2081		struct list_head *ima_rules_tmp;
  2082	
  2083		if (!list_empty(&ima_temp_rules)) {
  2084			list_for_each_entry(entry, &ima_temp_rules, list) {
  2085				if (ima_rules_equal(entry, new_rule))
  2086					return true;
  2087			}
  2088		}
  2089	
  2090		rcu_read_lock();
  2091		ima_rules_tmp = rcu_dereference(ima_rules);
  2092		list_for_each_entry_rcu(entry, ima_rules_tmp, list) {
  2093			if (ima_rules_equal(entry, new_rule)) {
  2094				rcu_read_unlock();
  2095				return true;
  2096			}
  2097		}
  2098		rcu_read_unlock();
  2099	
  2100		return false;
  2101	}
  2102	

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

^ permalink raw reply

* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Mimi Zohar @ 2025-11-06 22:15 UTC (permalink / raw)
  To: Coiby Xu
  Cc: Paul Moore, linux-integrity, linux-security-module, Karel Srot,
	James Morris, Serge E. Hallyn, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
	Eric Snowberg, open list, open list:MODULE SUPPORT
In-Reply-To: <d24wnmefebnheerigmh6ts5yskkutz726l6a2f6g5s3s5fhhrv@osaactobwb5g>

On Thu, 2025-11-06 at 21:29 +0800, Coiby Xu wrote:
> On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
> > On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
> [...]
> > 
> > Hi Coiby,
> > 
> > Based on the conversation with Paul, there is no reason to remove the existing
> > security_kernel_post_read_file() call.
> > 
> > The changes are similar to the 2nd link, but a bit different.
> > - Define a single enumeration named READING_MODULE_COMPRESSED.
> > 
> > - In module/main.c add a new security_kernel_post_read_file() call immediately
> > after decompressing the kernel module.  Like a previous version of this patch,
> > call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
> > based on MODULE_INIT_COMPRESSED_FILE.
> > 
> > - In ima_post_read_file() defer verifying the signature when the enumeration is
> > READING_MODULE_COMPRESSED.  (No need for a new function ima_read_kernel_module.)
> 
> Hi Mimi,
> 
> Thanks for summarizing your conversation with Paul! I can confirm Paul's
> approach works
> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
> 
> While testing the patch today, I realized there is another
> issue/challenge introduced by in-kernel module decompression. IMA
> appraisal is to verify the digest of compressed kernel module but
> currently the passed buffer is uncompressed module. When IMA uses
> uncompressed module data to calculate the digest, xattr signature
> verification will fail. If we always make IMA read the original kernel
> module data again to calculate the digest, does it look like a
> quick-and-dirty fix? If we can assume people won't load kernel module so
> often, the performance impact is negligible. Otherwise we may have to
> introduce a new LSM hook so IMA can access uncompressed and original
> module data one time.

ima_collect_measurement() stores the file hash info in the iint and uses that
information to verify the signature as stored in the security xattr. 
Decompressing the kernel module shouldn't affect the xattr signature
verification.

The patch with a few minor changes looks good:

- READDING_MODULE_CHECK -> READING_MODULE_CHECK
- Fix the enumeration name in ima_main.c
- scripts/checkpatch.pl code/comment line length has been relaxed to 100 chars,
but the section "Breaking long lines and strings" in
Documentation/process/coding-style.rst still recommends 80 characters.

There are cases where it is necessary to go over the 80 char line limit for
readability, but in general both Roberto and I prefer, as much as possible, to
limit the line length to 80 char.  To detect where/when the line limit is
greater than 80 chars, use the scripts/checkpatch.pl "--max-line-length=80"
option.

After fixing the patch, please post it to linux-integrity mailing list.

-- 
thanks,

Mimi

^ permalink raw reply

* Re: [Patch V1] ima: avoid duplicate policy rules insertions
From: Anirudh Venkataramanan @ 2025-11-06 20:32 UTC (permalink / raw)
  To: Tahera Fahimi, zohar, roberto.sassu, dmitry.kasatkin,
	eric.snowberg, paul, jmorris, serge, linux-integrity,
	linux-security-module, linux-kernel, code
In-Reply-To: <20251106181404.3429710-1-taherafahimi@linux.microsoft.com>

On 11/6/2025 10:14 AM, Tahera Fahimi wrote:
> Prevent redundant IMA policy rules by checking for duplicates before insertion. This ensures that
> rules are not re-added when userspace is restarted (using systemd-soft-reboot) without a full system
> reboot. ima_rule_exists() detects duplicates in both temporary and active rule lists.

I have run into this too. Thanks for proposing a patch!

FWIW - I am fairly new to the IMA subsystem, so feedback below is mostly 
structural, with some IMA specific comments.

> 
> Signed-off-by: Tahera Fahimi <taherafahimi@linux.microsoft.com>
> ---
>   security/integrity/ima/ima_policy.c | 157 +++++++++++++++++++++++++++-
>   1 file changed, 156 insertions(+), 1 deletion(-)
> 
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 164d62832f8ec..3dd902101dbda 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -1953,6 +1953,153 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>   	return result;
>   }
>   
> +static bool template_has_field(const char *field_id, const struct ima_template_desc *template2)
> +{
> +	int j;

j is declared in the loop header below too, which is more correct 
because it keeps the scope of j to be within the loop. So I'd say get 
rid of the above declaration.

> +
> +	for (int j = 0; j < template2->num_fields; j++)
> +		if (strcmp(field_id, template2->fields[j]->field_id) == 0)
> +			return true;
I believe the preferred kernel style is to use if (!strcmp(...)).

> +
> +	return false;
> +}
> +
> +static bool keyring_has_item(const char *item, const struct ima_rule_opt_list *keyrings)
> +{
> +	int j;
> +
> +	for (j = 0; j < keyrings->count; j++) {
> +		if (strcmp(item, keyrings->items[j]) == 0)
> +			return true;
> +	}
> +	return false;
> +}
> +
> +static bool labels_has_item(const char *item, const struct ima_rule_opt_list *labels)
> +{
> +	int j;
> +
> +	for (j = 0; j < labels->count; j++) {
> +		if (strcmp(item, labels->items[j]) == 0)
> +			return true;
> +	}
> +	return false;
> +}
> +
> +static bool ima_rules_equal(const struct ima_rule_entry *rule1, const struct ima_rule_entry *rule2)
> +{
> +	int i;

i is used further down in this function, and even in all those cases, 
the scope of i can be limited to the loop body where it's used.

If you didn't know this already - you can use cppcheck to identify and 
reduce the scope of variables.

> +
> +	if (rule1->flags != rule2->flags)
> +		return false;
> +
> +	if (rule1->action != rule2->action)
> +		return false;
> +
> +	if (((rule1->flags & IMA_FUNC) && rule1->func != rule2->func) ||
> +	    ((rule1->flags & (IMA_MASK | IMA_INMASK)) && rule1->mask != rule2->mask) ||
> +	    ((rule1->flags & IMA_FSMAGIC) && rule1->fsmagic != rule2->fsmagic) ||
> +	    ((rule1->flags & IMA_FSUUID) && !uuid_equal(&rule1->fsuuid, &rule2->fsuuid)) ||
> +	    ((rule1->flags & IMA_UID) && !uid_eq(rule1->uid, rule2->uid)) ||
> +	    ((rule1->flags & IMA_GID) && !gid_eq(rule1->gid, rule2->gid)) ||
> +	    ((rule1->flags & IMA_FOWNER) && !uid_eq(rule1->fowner, rule2->fowner)) ||
> +	    ((rule1->flags & IMA_FGROUP) && !gid_eq(rule1->fgroup, rule2->fgroup)) ||
> +	    ((rule1->flags & IMA_FSNAME) && (strcmp(rule1->fsname, rule2->fsname) != 0)) ||
> +	    ((rule1->flags & IMA_PCR) && rule1->pcr != rule2->pcr) ||
> +	    ((rule1->flags & IMA_VALIDATE_ALGOS) &&
> +	      rule1->allowed_algos != rule2->allowed_algos) ||
> +	    ((rule1->flags & IMA_EUID) && !uid_eq(rule1->uid, rule2->uid)) ||
> +	    ((rule1->flags & IMA_EGID) && !gid_eq(rule1->gid, rule2->gid)))
> +		return false;

So the goal is to prevent the exact same policy rule from being added, 
not to update an existing rule, correct? IOW, you could end up with two 
very similar rules, because the new rule has one thing that's different 
compared to the existing rule?

I feel that a little bit of commentary around what makes two rules the 
same would be useful.

> +
> +	if (!rule1->template && !rule2->template) {
> +		;
You're trying to do nothing and continue on. A goto statement would 
communicate intent better. There are other places below with the same 
noop structure.

To be fair, I also don't completely understand what you're trying to 
achieve here, Regardless, this "do nothing inside a conditional" looks 
weird and I feel like there should be a way to structure your logic 
without resorting to this.

> +	} else if (!rule1->template || !rule2->template) {
> +		return false;
> +	} else if (rule1->template->num_fields != rule2->template->num_fields) {
> +		return false;
> +	} else if (rule1->template->num_fields != 0) {
> +		for (i = 0; i < rule1->template->num_fields; i++) {
> +			if (!template_has_field(rule1->template->fields[i]->field_id,
> +						rule2->template))
> +				return false;
> +		}
> +	}

if + return will achieve the same end goals as else if + return, with 
lesser clutter. I have seen some static analyzers flag this pattern, but 
I can't remember which one at the moment.

So something like this:

if (!rule1->template && !rule2->template)
     goto some_target;

if (!rule1->template || !rule2->template)
     return false;

if (rule1->template->num_fields != rule2->template->num_fields)
     return false;

if (rule1->template->num_fields != 0) {
     for (i = 0; i < rule1->template->num_fields; i++) {
         if (!template_has_field(rule1->template->fields[i]->field_id,
                                 rule2->template))
               return false;
         }
}

some_target:
...
...

> +
> +	if (rule1->flags & IMA_KEYRINGS) {
> +		if (!rule1->keyrings && !rule2->keyrings) {
> +			;

Another if block no-op

> +		} else if (!rule1->keyrings || !rule2->keyrings) {
> +			return false;
> +		} else if (rule1->keyrings->count != rule2->keyrings->count) {
> +			return false;
> +		} else if (rule1->keyrings->count != 0) {

if (rule1->keyrings->count)

> +			for (i = 0; i < rule1->keyrings->count; i++) {

for (int i,

> +				if (!keyring_has_item(rule1->keyrings->items[i], rule2->keyrings))
> +					return false;
> +			}
> +		}
> +	}
> +
> +	if (rule1->flags & IMA_LABEL) {
> +		if (!rule1->label && !rule2->label) {
> +			;

Another if block no-op

> +		} else if (!rule1->label || !rule2->label) {
> +			return false;
> +		} else if (rule1->label->count != rule2->label->count) {
> +			return false;
> +		} else if (rule1->label->count != 0) {
> +			for (i = 0; i < rule1->label->count; i++) {
> +				if (!labels_has_item(rule1->label->items[i], rule2->label))
> +					return false;
> +			}
> +		}
> +	}
> +
> +	for (i = 0; i < MAX_LSM_RULES; i++) {

for (int i,

> +		if (!rule1->lsm[i].rule && !rule2->lsm[i].rule)
> +			continue;
> +
> +		if (!rule1->lsm[i].rule || !rule2->lsm[i].rule)
> +			return false;
> +
> +		if (strcmp(rule1->lsm[i].args_p, rule2->lsm[i].args_p) != 0)
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
> +/**
> + * ima_rule_exists - check if a rule already exists in the policy
> + *
> + * Checking both the active policy and the temporary rules list.
> + */
> +static bool ima_rule_exists(struct ima_rule_entry *new_rule)
> +{
> +	struct ima_rule_entry *entry;
> +	struct list_head *ima_rules_tmp;
> +
> +	if (!list_empty(&ima_temp_rules)) {
> +		list_for_each_entry(entry, &ima_temp_rules, list) {
> +			if (ima_rules_equal(entry, new_rule))
> +				return true;
> +		}
> +	}
> +
> +	rcu_read_lock();
> +	ima_rules_tmp = rcu_dereference(ima_rules);
> +	list_for_each_entry_rcu(entry, ima_rules_tmp, list) {
> +		if (ima_rules_equal(entry, new_rule)) {
> +			rcu_read_unlock();
> +			return true;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return false;
> +}
> +
>   /**
>    * ima_parse_add_rule - add a rule to ima_policy_rules
>    * @rule: ima measurement policy rule
> @@ -1993,7 +2140,15 @@ ssize_t ima_parse_add_rule(char *rule)
>   		return result;
>   	}
>   
> -	list_add_tail(&entry->list, &ima_temp_rules);
> +	if (!ima_rule_exists(entry)) {
> +		list_add_tail(&entry->list, &ima_temp_rules);
> +	} else {
> +		result = -EEXIST;
Is it necessary to set result? Or can you just pass -EEXIST to the audit 
call below?

> +		ima_free_rule(entry);
> +		integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
> +				    NULL, op, "duplicate-policy", result,
> +				    audit_info);
> +	}
>   
>   	return len;
>   }


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox