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

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

Hello!

Thanks for the updated patch set!

On Tue, Jun 02, 2026 at 05:27: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.
> 
> Skip the recording only for the single-process target types, so the scope
> is enforced against every group member at delivery time.  The direct
> kill() path (hook_task_kill) already evaluates the live domain and is
> unaffected.

Consider the following scenario:

 - Processes P1 and P2 are in the same process group
 - Threads T2.1 and T2.2 are part of P2.
   - T2.1 is the thread group leader of P2.
   - T2.2 is in a signal-scoped Landlock domain
 - T2.2 registers the SIGIO for the entire PGID
 - Someone writes to the FD, triggering the SIGIO mechanism

What I would expect in this scenario is:

 - T2.1 receives the SIGIO because it is the thread group leader for
   P2.  (SIGIO with PGID only sends to one thread per process)
 - It is OK for it to receive the signal because signals between
   sibling threads should be permitted.
 - No other threads receive SIGIO.

I believe the result after this patch is:

 - No threads receive the SIGIO at all.

This is because we have been setting T2.2's Landlock domain as the
"sending domain" for the hook_file_sigiotask(), and that hook does on
its own not do the "same_thread_group()" check, and the thread group
leader T2.1 is outside of the T2.2's Landlock domain.


To be clear, the patch is still obviously an improvement, given that
it fixes a bypass for the signaling policy; it just seems to block it
slightly too broadly in this corner scenario?

The scenario does not happen *much* in practice, because SIGIO is not
used much, and starting with 7.0, multithreaded processes should
ideally use TSYNC and have their threads all in the exact same
Landlock domain.  (Before TSYNC, this only affected the case where a
process was already(!) multithreaded at the time of Landlock
enforcement.)

I like the simplicity of this fix, but I'm afraid it does not do 100%
the correct thing.  (I have not tried it out though and I'm happy to
stand corrected if my analysis is wrong.)

The fix would be for a very fringe scenario only, where multiple
conditions come together:

- An already multithreaded process enforcing a Landlock policy
- Not using the TSYNC flag for it (since Linux 7.0)
- Using SIGIO
- Using SIGIO with signaling to a full PGID, including the current process
- SIGIO registration happens from a non-thread-leader thread
- That thread is in a signal-scoped Landlock domain

Mickaël, maybe you have some thoughts on the tradeoff?

> 
> Fixes: 18eb75f3af40 ("landlock: Always allow signals between threads of the same process")
> Cc: stable@vger.kernel.org
> Tested-by: Justin Suess <utilityemal77@gmail.com>
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
>  security/landlock/fs.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index c1ecfe239032..2ebad70a956d 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.
> +	 */
> +	if (fown->pid_type != PIDTYPE_PID && fown->pid_type != PIDTYPE_TGID)
> +		return true;
> +
>  	return !same_thread_group(p, current);
>  }
>  
> -- 
> 2.43.0

Thanks,
–Günther

P.S: The threaded mail is now in the right format.  Remaining nit
     though: By convention, new patchset versions are posted at the
     top (no Reply-To header in the cover letter), and this is what
     many maintainers filter for - it is easier to get maintainers
     attention when sticking to that convention.