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

["Günther Noack" <gnoack3000@gmail.com>]

On Thu, Jun 04, 2026 at 11:16:56PM +0000, Bryam Vargas wrote:
> 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@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@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
> 
> 

Reviewed-by: Günther Noack <gnoack3000@gmail.com>

Thank you, this looks good!
–Günther