[PATCH v5 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass on the SIGIO path

[Bryam Vargas]

LANDLOCK_SCOPE_SIGNAL must prevent a sandboxed process from signaling
processes outside its Landlock domain.  It can be bypassed through the
asynchronous SIGIO delivery path.

A sandboxed process that owns any file or socket can arm it with
fcntl(F_SETOWN, fd, -pgid), fcntl(F_SETSIG, fd, SIGKILL) and O_ASYNC, so
that an I/O event makes the kernel deliver the chosen signal to the whole
process group.  As the head of its own process group -- the default right
after fork() -- that group also holds the non-sandboxed process that
launched it, e.g. a supervisor or a security monitor.  The sandbox can
thus kill or repeatedly signal exactly the processes SCOPE_SIGNAL is meant
to protect from it.

The scope is enforced in hook_file_send_sigiotask() against the Landlock
domain recorded at F_SETOWN time, not the live domain of the sender.
control_current_fowner() decides whether to record that domain and skips
recording it when the fowner target is in the caller's thread group --
safe only when the target is a single process sharing the caller's
credentials (PIDTYPE_PID, PIDTYPE_TGID).  For a process group
(PIDTYPE_PGID) the target resolves to the caller itself when it is the
group head, recording is skipped, and hook_file_send_sigiotask() then lets
the signal fan out to the whole group unchecked.

Record the domain for every non single-process target so the scope is
enforced against each group member at delivery time.

That recording is necessary but not sufficient on its own: the kernel
signals a process group through its members' thread-group leaders, and the
leader of the registrant's own process can carry a different Landlock
domain than the sibling thread that armed the owner.  domain_is_scoped()
would then deny that leader, even though commit 18eb75f3af40 ("landlock:
Always allow signals between threads of the same process") requires
same-process delivery to be allowed.  hook_task_kill() avoids this by
evaluating same_thread_group() live, per recipient; the SIGIO path instead
delegates the whole decision to a single registration-time check, which a
process-group fan-out cannot honor.

So also record the registrant's thread group next to its domain and exempt
it at delivery: hook_file_send_sigiotask() allows the signal whenever the
recipient belongs to the registrant's own process, restoring the
same-process guarantee while keeping out-of-domain group members blocked.
The direct kill() path (hook_task_kill) already evaluates the live domain
and is unaffected.

Fixes: 18eb75f3af40 ("landlock: Always allow signals between threads of the same process")
Cc: stable at vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity at proton.me>
---
 security/landlock/fs.c   | 15 +++++++++++++++
 security/landlock/fs.h   | 10 ++++++++++
 security/landlock/task.c | 11 +++++++++++
 3 files changed, 36 insertions(+)

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index c1ecfe239032..ff2c12e38bfc 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1909,6 +1909,15 @@ static bool control_current_fowner(struct fown_struct *const fown)
 	if (!p)
 		return true;
 
+	/*
+	 * A process-group fowner fans the signal out to every member at
+	 * delivery time, so record the domain for any non single-process
+	 * target -- even when it resolves to current as the group head -- and
+	 * let hook_file_send_sigiotask() check the live scope per recipient.
+	 */
+	if (fown->pid_type != PIDTYPE_PID && fown->pid_type != PIDTYPE_TGID)
+		return true;
+
 	return !same_thread_group(p, current);
 }
 
@@ -1916,6 +1925,7 @@ static void hook_file_set_fowner(struct file *file)
 {
 	struct landlock_ruleset *prev_dom;
 	struct landlock_cred_security fown_subject = {};
+	struct pid *prev_tg, *fown_tg = NULL;
 	size_t fown_layer = 0;
 
 	if (control_current_fowner(file_f_owner(file))) {
@@ -1928,21 +1938,26 @@ static void hook_file_set_fowner(struct file *file)
 		if (new_subject) {
 			landlock_get_ruleset(new_subject->domain);
 			fown_subject = *new_subject;
+			fown_tg = get_pid(task_tgid(current));
 		}
 	}
 
 	prev_dom = landlock_file(file)->fown_subject.domain;
+	prev_tg = landlock_file(file)->fown_tg;
 	landlock_file(file)->fown_subject = fown_subject;
+	landlock_file(file)->fown_tg = fown_tg;
 #ifdef CONFIG_AUDIT
 	landlock_file(file)->fown_layer = fown_layer;
 #endif /* CONFIG_AUDIT*/
 
 	/* May be called in an RCU read-side critical section. */
 	landlock_put_ruleset_deferred(prev_dom);
+	put_pid(prev_tg);
 }
 
 static void hook_file_free_security(struct file *file)
 {
+	put_pid(landlock_file(file)->fown_tg);
 	landlock_put_ruleset_deferred(landlock_file(file)->fown_subject.domain);
 }
 
diff --git a/security/landlock/fs.h b/security/landlock/fs.h
index bf9948941f2f..911b83669e20 100644
--- a/security/landlock/fs.h
+++ b/security/landlock/fs.h
@@ -78,6 +78,16 @@ struct landlock_file_security {
 	 * euid.
 	 */
 	struct landlock_cred_security fown_subject;
+	/**
+	 * @fown_tg: Thread group of the task that set the file owner, pinned
+	 * while @fown_subject holds a domain.  It lets
+	 * hook_file_send_sigiotask() always allow a SIGIO delivered to the
+	 * owner's own process -- e.g. the thread-group leader reached through a
+	 * process-group owner -- matching the same-process exemption of
+	 * hook_task_kill().  NULL when no domain is recorded.  Protected by
+	 * file->f_owner->lock, like @fown_subject.
+	 */
+	struct pid *fown_tg;
 };
 
 #ifdef CONFIG_AUDIT
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 6d46042132ce..7ddf211f75c3 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -411,6 +411,17 @@ static int hook_file_send_sigiotask(struct task_struct *tsk,
 	if (!subject->domain)
 		return 0;
 
+	/*
+	 * Always allow delivery to the file owner's own process, including a
+	 * thread-group leader reached through a process-group owner.  This
+	 * mirrors hook_task_kill()'s same-process exemption and preserves the
+	 * guarantee of commit 18eb75f3af40 ("landlock: Always allow signals
+	 * between threads of the same process"), which the registration-time
+	 * check cannot honor for a process-group target.
+	 */
+	if (task_tgid(tsk) == landlock_file(fown->file)->fown_tg)
+		return 0;
+
 	scoped_guard(rcu)
 	{
 		is_scoped = domain_is_scoped(subject->domain,
-- 
2.43.0