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

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

[Günther Noack]

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.

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

[Bryam Vargas]

Hi Günther,

> 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 [...]

Confirmed -- I traced the delivery path and your analysis holds.

For a PGID owner the signal is anchored per process on its thread-group
leader: a task is attached to pid->tasks[PIDTYPE_PGID] only in the
thread_group_leader() branch of copy_process(), so send_sigio()'s
do_each_pid_task(pid, PIDTYPE_PGID, p) walk visits exactly T2.1 for P2,
never the non-leader T2.2.  hook_file_send_sigiotask() then runs
domain_is_scoped(recorded T2.2 domain, T2.1's live domain, SIGNAL) and,
having no same_thread_group() exemption of its own (unlike
hook_task_kill()), denies it -- even though T2.1 and T2.2 share P2's
signal_struct and 18eb75f3af40 mandates that same-process delivery always
be allowed.  T2.1 is P2's only entry on the PGID list, so P2 receives
nothing.  You are right.

One thing worth putting on the record: this over-block is not introduced
by the patch.  In unpatched control_current_fowner() the PGID case already
resolves through pid_task(fown->pid, PIDTYPE_PGID), which returns an
arbitrary hlist head -- one representative leader.  Whenever that head is
outside the caller's thread group, the domain is already recorded today and
the same delivery-time denial of the registrant's own leader already fires.
The patch only makes domain recording for PGID unconditional, i.e. it turns
that order-dependent behaviour into a deterministic one while closing the
order-dependent bypass.  So the corner you describe is a pre-existing gap in
the delivery hook, not a regression in v4.

That points at the real root cause: same_thread_group is a *per-recipient*
property, but control_current_fowner() approximates it once, at F_SETOWN
time, against a single pid_task() representative.  hook_task_kill() gets
this right because it evaluates same_thread_group(p, current) live, per
actual recipient.  hook_file_send_sigiotask() is the SIGIO analogue but
delegates the whole thread-group decision to that one registration-time
check, which a PGID delivery set simply cannot be captured by.

So the fully-correct fix is to move the same-process exemption to delivery
time, keyed to the *registrant* rather than to current (at SIGIO time
current is the fd writer, not the task that armed F_SETOWN).  Concretely:
when hook_file_set_fowner() records the domain, also pin
get_pid(task_tgid(current)) in struct landlock_file_security; in
hook_file_send_sigiotask(), before domain_is_scoped(), return 0 when
task_tgid(tsk) == that recorded pid.  PGID owners still record the domain
(so P1 stays blocked -- the bypass fix), but the registrant's own process,
including T2.1, is always allowed -- restoring 18eb75f3af40 exactly.  The
new pid is taken/put in lockstep with fown_subject.domain under the same
file->f_owner->lock and freed in hook_file_free_security(); the equality
test follows neither pid, so there is no extra RCU surface.  Sketch:

    /* struct landlock_file_security */
    struct pid *fown_tg;   /* registrant's thread group; NULL if no domain */

    /* hook_file_set_fowner(), where fown_subject is recorded */
    fown_tg = get_pid(task_tgid(current));
    ...
    put_pid(landlock_file(file)->fown_tg);     /* release previous */
    landlock_file(file)->fown_tg = fown_tg;

    /* hook_file_free_security() */
    put_pid(landlock_file(file)->fown_tg);

    /* hook_file_send_sigiotask(), after the !subject->domain quick return */
    if (task_tgid(tsk) == landlock_file(fown->file)->fown_tg)
            return 0;   /* same process as the registrant: always allowed */

I do not see a correct fix that avoids recording the registrant's identity:
the registrant task is deliberately discarded after set_fowner (only its
domain is kept), and exempting on a shared *domain* instead would be
insecure -- sibling threads can hold different domains, and a different
process could share one.

> To be clear, the patch is still obviously an improvement [...] it just
> seems to block it slightly too broadly in this corner scenario?
> [...] Mickaël, maybe you have some thoughts on the tradeoff?

Agreed on both counts.  Mickaël -- two ways to land this:

  (a) keep v4 as is.  It closes the bypass; the residual same-process
      over-block is pre-existing, deterministic only under the stacked
      conditions Günther listed (already-multithreaded enforce, no TSYNC,
      SIGIO to a PGID that includes self, registered from a non-leader
      thread in a per-thread signal-scoped domain), and arguably tolerable.

  (b) v5 = v4 + the delivery-time exemption above.  Strictly more correct:
      it also closes the pre-existing delivery-hook gap and restores
      18eb75f3af40's same-process invariant, at the cost of one struct pid*
      in landlock_file_security.

I lean (b) -- it fixes the actual root cause rather than the one reachable
instance -- and I am happy to spin it (with an added selftest covering the
PGID-includes-self / non-leader-registrant case, A/B verified) or to hold at
v4 if you would rather keep the change minimal.  Your call on whether the
corner warrants the extra state.

> P.S: [...] new patchset versions are posted at the top (no Reply-To
>      header in the cover letter) [...]

Will do -- v5 (whichever option) goes out as a fresh top-level thread, no
In-Reply-To/Reply-To pointing back at this review.

Bryam

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

[Günther Noack]

Hello Bryam,

Just a brief mail to confirm the approach; this makes sense to me.

On Thu, Jun 04, 2026 at 10:27:13AM +0000, Bryam Vargas wrote:
> > 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 [...]
> 
> Confirmed -- I traced the delivery path and your analysis holds.
> 
> For a PGID owner the signal is anchored per process on its thread-group
> leader: a task is attached to pid->tasks[PIDTYPE_PGID] only in the
> thread_group_leader() branch of copy_process(), so send_sigio()'s
> do_each_pid_task(pid, PIDTYPE_PGID, p) walk visits exactly T2.1 for P2,
> never the non-leader T2.2.  hook_file_send_sigiotask() then runs
> domain_is_scoped(recorded T2.2 domain, T2.1's live domain, SIGNAL) and,
> having no same_thread_group() exemption of its own (unlike
> hook_task_kill()), denies it -- even though T2.1 and T2.2 share P2's
> signal_struct and 18eb75f3af40 mandates that same-process delivery always
> be allowed.  T2.1 is P2's only entry on the PGID list, so P2 receives
> nothing.  You are right.
> 
> One thing worth putting on the record: this over-block is not introduced
> by the patch.  In unpatched control_current_fowner() the PGID case already
> resolves through pid_task(fown->pid, PIDTYPE_PGID), which returns an
> arbitrary hlist head -- one representative leader.  Whenever that head is
> outside the caller's thread group, the domain is already recorded today and
> the same delivery-time denial of the registrant's own leader already fires.
> The patch only makes domain recording for PGID unconditional, i.e. it turns
> that order-dependent behaviour into a deterministic one while closing the
> order-dependent bypass.  So the corner you describe is a pre-existing gap in
> the delivery hook, not a regression in v4.
> 
> That points at the real root cause: same_thread_group is a *per-recipient*
> property, but control_current_fowner() approximates it once, at F_SETOWN
> time, against a single pid_task() representative.  hook_task_kill() gets
> this right because it evaluates same_thread_group(p, current) live, per
> actual recipient.  hook_file_send_sigiotask() is the SIGIO analogue but
> delegates the whole thread-group decision to that one registration-time
> check, which a PGID delivery set simply cannot be captured by.
> 
> So the fully-correct fix is to move the same-process exemption to delivery
> time, keyed to the *registrant* rather than to current (at SIGIO time
> current is the fd writer, not the task that armed F_SETOWN).  Concretely:
> when hook_file_set_fowner() records the domain, also pin
> get_pid(task_tgid(current)) in struct landlock_file_security; in
> hook_file_send_sigiotask(), before domain_is_scoped(), return 0 when
> task_tgid(tsk) == that recorded pid.  PGID owners still record the domain
> (so P1 stays blocked -- the bypass fix), but the registrant's own process,
> including T2.1, is always allowed -- restoring 18eb75f3af40 exactly.  The
> new pid is taken/put in lockstep with fown_subject.domain under the same
> file->f_owner->lock and freed in hook_file_free_security(); the equality
> test follows neither pid, so there is no extra RCU surface.  Sketch:
> 
>     /* struct landlock_file_security */
>     struct pid *fown_tg;   /* registrant's thread group; NULL if no domain */
> 
>     /* hook_file_set_fowner(), where fown_subject is recorded */
>     fown_tg = get_pid(task_tgid(current));
>     ...
>     put_pid(landlock_file(file)->fown_tg);     /* release previous */
>     landlock_file(file)->fown_tg = fown_tg;
> 
>     /* hook_file_free_security() */
>     put_pid(landlock_file(file)->fown_tg);
> 
>     /* hook_file_send_sigiotask(), after the !subject->domain quick return */
>     if (task_tgid(tsk) == landlock_file(fown->file)->fown_tg)
>             return 0;   /* same process as the registrant: always allowed */
n> 
> I do not see a correct fix that avoids recording the registrant's identity:
> the registrant task is deliberately discarded after set_fowner (only its
> domain is kept), and exempting on a shared *domain* instead would be
> insecure -- sibling threads can hold different domains, and a different
> process could share one.

Yes, your approach checks out for me; I also think that storing this
additional information is the best approach; we need to know during
hook_file_send_sigiotask() what the TGID of the registering task was,
in order to tell apart signals within the same process from signals
going outwards of that process.


> > To be clear, the patch is still obviously an improvement [...] it just
> > seems to block it slightly too broadly in this corner scenario?
> > [...] Mickaël, maybe you have some thoughts on the tradeoff?
> 
> Agreed on both counts.  Mickaël -- two ways to land this:
> 
>   (a) keep v4 as is.  It closes the bypass; the residual same-process
>       over-block is pre-existing, deterministic only under the stacked
>       conditions Günther listed (already-multithreaded enforce, no TSYNC,
>       SIGIO to a PGID that includes self, registered from a non-leader
>       thread in a per-thread signal-scoped domain), and arguably tolerable.
> 
>   (b) v5 = v4 + the delivery-time exemption above.  Strictly more correct:
>       it also closes the pre-existing delivery-hook gap and restores
>       18eb75f3af40's same-process invariant, at the cost of one struct pid*
>       in landlock_file_security.
> 
> I lean (b) -- it fixes the actual root cause rather than the one reachable
> instance -- and I am happy to spin it (with an added selftest covering the
> PGID-includes-self / non-leader-registrant case, A/B verified) or to hold at
> v4 if you would rather keep the change minimal.  Your call on whether the
> corner warrants the extra state.

+1, I also think that the approach is quite clean.  Some checks would
happen at a later time, but it seems unavoidable in the generic case.
Checking TGID during hook_file_send_sigiotask() sounds reasonably
cheap.  (I suspect that trying to do that check early during
hook_file_set_fowner() would not save us much.)


> > P.S: [...] new patchset versions are posted at the top (no Reply-To
> >      header in the cover letter) [...]
> 
> Will do -- v5 (whichever option) goes out as a fresh top-level thread, no
> In-Reply-To/Reply-To pointing back at this review.

Awesome, thank you very much for looking into patching this! :)

–Günther