landlock: Always allow signals between threads of the same process

JIRA: https://issues.redhat.com/browse/RHEL-125143

Upstream Status: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

Conflicts: Replaced file_f_owner due to missing	1934b21
	("file: reclaim 24 bytes from f_owner").
	Minor context difference.

commit 18eb75f
Author: Mickaël Salaün <mic@digikod.net>
Date:   Tue Mar 18 17:14:40 2025 +0100

    landlock: Always allow signals between threads of the same process

    Because Linux credentials are managed per thread, user space relies on
    some hack to synchronize credential update across threads from the same
    process.  This is required by the Native POSIX Threads Library and
    implemented by set*id(2) wrappers and libcap(3) to use tgkill(2) to
    synchronize threads.  See nptl(7) and libpsx(3).  Furthermore, some
    runtimes like Go do not enable developers to have control over threads
    [1].

    To avoid potential issues, and because threads are not security
    boundaries, let's relax the Landlock (optional) signal scoping to always
    allow signals sent between threads of the same process.  This exception
    is similar to the __ptrace_may_access() one.

    hook_file_set_fowner() now checks if the target task is part of the same
    process as the caller.  If this is the case, then the related signal
    triggered by the socket will always be allowed.

    Scoping of abstract UNIX sockets is not changed because kernel objects
    (e.g. sockets) should be tied to their creator's domain at creation
    time.

    Note that creating one Landlock domain per thread puts each of these
    threads (and their future children) in their own scope, which is
    probably not what users expect, especially in Go where we do not control
    threads.  However, being able to drop permissions on all threads should
    not be restricted by signal scoping.  We are working on a way to make it
    possible to atomically restrict all threads of a process with the same
    domain [2].

    Add erratum for signal scoping.

    Closes: landlock-lsm/go-landlock#36
    Fixes: 54a6e6b ("landlock: Add signal scoping")
    Fixes: c899496 ("selftests/landlock: Test signal scoping for threads")
    Depends-on: 26f2043 ("fs: Fix file_set_fowner LSM hook inconsistencies")
    Link: https://pkg.go.dev/kernel.org/pub/linux/libs/security/libcap/psx [1]
    Link: landlock-lsm/linux#2 [2]
    Cc: Günther Noack <gnoack@google.com>
    Cc: Paul Moore <paul@paul-moore.com>
    Cc: Serge Hallyn <serge@hallyn.com>
    Cc: Tahera Fahimi <fahimitahera@gmail.com>
    Cc: stable@vger.kernel.org
    Acked-by: Christian Brauner <brauner@kernel.org>
    Link: https://lore.kernel.org/r/20250318161443.279194-6-mic@digikod.net
    [mic: Add extra pointer check and RCU guard, and ease backport]
    Signed-off-by: Mickaël Salaün <mic@digikod.net>

Signed-off-by: Štěpán Horáček <shoracek@redhat.com>

Author: shoracek

security/landlock/errata/abi-6.h

@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+/**
+ * DOC: erratum_2
+ *
+ * Erratum 2: Scoped signal handling
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This fix addresses an issue where signal scoping was overly restrictive,
+ * preventing sandboxed threads from signaling other threads within the same
+ * process if they belonged to different domains.  Because threads are not
+ * security boundaries, user space might assume that any thread within the same
+ * process can send signals between themselves (see :manpage:`nptl(7)` and
+ * :manpage:`libpsx(3)`).  Consistent with :manpage:`ptrace(2)` behavior, direct
+ * interaction between threads of the same process should always be allowed.
+ * This change ensures that any thread is allowed to send signals to any other
+ * thread within the same process, regardless of their domain.
+ */
+LANDLOCK_ERRATUM(2)

security/landlock/fs.c

@@ -27,7 +27,9 @@
 #include <linux/mount.h>
 #include <linux/namei.h>
 #include <linux/path.h>
+#include <linux/pid.h>
 #include <linux/rcupdate.h>
+#include <linux/sched/signal.h>
 #include <linux/spinlock.h>
 #include <linux/stat.h>
 #include <linux/types.h>
@@ -1636,21 +1638,46 @@ static int hook_file_ioctl_compat(struct file *file, unsigned int cmd,
 	return -EACCES;
 }
 
-static void hook_file_set_fowner(struct file *file)
+/*
+ * Always allow sending signals between threads of the same process.  This
+ * ensures consistency with hook_task_kill().
+ */
+static bool control_current_fowner(struct fown_struct *const fown)
 {
-	struct landlock_ruleset *new_dom, *prev_dom;
+	struct task_struct *p;
 
 	/*
 	 * Lock already held by __f_setown(), see commit 26f204380a3c ("fs: Fix
 	 * file_set_fowner LSM hook inconsistencies").
 	 */
-	lockdep_assert_held(&file->f_owner.lock);
-	new_dom = landlock_get_current_domain();
-	landlock_get_ruleset(new_dom);
+	lockdep_assert_held(&fown->lock);
+
+	/*
+	 * Some callers (e.g. fcntl_dirnotify) may not be in an RCU read-side
+	 * critical section.
+	 */
+	guard(rcu)();
+	p = pid_task(fown->pid, fown->pid_type);
+	if (!p)
+		return true;
+
+	return !same_thread_group(p, current);
+}
+
+static void hook_file_set_fowner(struct file *file)
+{
+	struct landlock_ruleset *prev_dom;
+	struct landlock_ruleset *new_dom = NULL;
+
+	if (control_current_fowner(&file->f_owner)) {
+		new_dom = landlock_get_current_domain();
+		landlock_get_ruleset(new_dom);
+	}
+
 	prev_dom = landlock_file(file)->fown_domain;
 	landlock_file(file)->fown_domain = new_dom;
 
-	/* Called in an RCU read-side critical section. */
+	/* May be called in an RCU read-side critical section. */
 	landlock_put_ruleset_deferred(prev_dom);
 }
 

security/landlock/task.c

@@ -13,6 +13,7 @@
 #include <linux/lsm_hooks.h>
 #include <linux/rcupdate.h>
 #include <linux/sched.h>
+#include <linux/sched/signal.h>
 #include <net/af_unix.h>
 #include <net/sock.h>
 
@@ -254,6 +255,17 @@ static int hook_task_kill(struct task_struct *const p,
 		/* Dealing with USB IO. */
 		dom = landlock_cred(cred)->domain;
 	} else {
+		/*
+		 * Always allow sending signals between threads of the same process.
+		 * This is required for process credential changes by the Native POSIX
+		 * Threads Library and implemented by the set*id(2) wrappers and
+		 * libcap(3) with tgkill(2).  See nptl(7) and libpsx(3).
+		 *
+		 * This exception is similar to the __ptrace_may_access() one.
+		 */
+		if (same_thread_group(p, current))
+			return 0;
+
 		dom = landlock_get_current_domain();
 	}
 

tools/testing/selftests/landlock/scoped_signal_test.c

@@ -281,7 +281,7 @@ TEST(signal_scoping_threads)
 	/* Restricts the domain after creating the first thread. */
 	create_scoped_domain(_metadata, LANDLOCK_SCOPE_SIGNAL);
 
-	ASSERT_EQ(EPERM, pthread_kill(no_sandbox_thread, 0));
+	ASSERT_EQ(0, pthread_kill(no_sandbox_thread, 0));
 	ASSERT_EQ(1, write(thread_pipe[1], ".", 1));
 
 	ASSERT_EQ(0, pthread_create(&scoped_thread, NULL, thread_func, NULL));