Linux Security Modules development
 help / color / mirror / Atom feed
* [PATCH v3 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass via F_SETOWN to invoker's pgid
@ 2026-05-29 19:07 hexlabsecurity
  2026-05-31 10:41 ` Mickaël Salaün
  2026-06-01 22:08 ` [PATCH v3 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass via F_SETOWN to invoker's pgid Günther Noack
  0 siblings, 2 replies; 7+ messages in thread
From: hexlabsecurity @ 2026-05-29 19:07 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Justin Suess, gnoack@google.com,
	linux-security-module@vger.kernel.org, stable@vger.kernel.org

From b5fdc79ce1cb2881d59dfed01d3d9170306be9e8 Mon Sep 17 00:00:00 2001
From: Bryam Vargas <hexlabsecurity@proton.me>
Date: Fri, 29 May 2026 12:49:41 -0500
Subject: [PATCH v3 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass via
 F_SETOWN to invoker's pgid

A Landlock-restricted process can bypass LANDLOCK_SCOPE_SIGNAL on the
SIGIO delivery path and deliver arbitrary signals (including SIGKILL via
F_SETSIG) to non-Landlocked targets that share its pgid, by exploiting a
producer-side cache-vs-live evaluation gap.

The SIGIO path in hook_file_send_sigiotask() consults a cached subject
stored in landlock_file(file)->fown_subject at fcntl(F_SETOWN) time
(via hook_file_set_fowner()), instead of evaluating the live Landlock
domain of the invoking task at signal-send time. The capture is gated
by control_current_fowner(), which returns false (skipping capture)
when pid_task(fown->pid, fown->pid_type) is in current's thread group.

This is correct for PIDTYPE_TGID / PIDTYPE_PID, where the target is a
single task sharing current's cred. It is unsafe for PIDTYPE_PGID and
PIDTYPE_SID: when current is at the head of its pgid hlist -- the
default placement after fork(), hlist_add_head_rcu() in kernel/fork.c --
pid_task(pgid, PIDTYPE_PGID) resolves to current itself,
same_thread_group(current, current) is true, the capture is skipped, and
fown_subject.domain stays NULL. hook_file_send_sigiotask() then
short-circuits at "if (!subject->domain) return 0;", letting the kernel
fan the signal out to every member of the group, including tasks outside
current's Landlock domain that SCOPE_SIGNAL is supposed to protect.

The direct kill() path (hook_task_kill) is unaffected: it evaluates
current's live domain on every call. Only the cached SIGIO path is
broken.

Tighten control_current_fowner() to apply the thread-group exemption
only when the target identifies a single task whose Landlock cred is
necessarily shared with current (PIDTYPE_TGID, PIDTYPE_PID). For
PIDTYPE_PGID and PIDTYPE_SID, always capture the current Landlock
subject so the consumer's scope check runs against every member of the
group at delivery time.

Stable kernels before the fown_subject conversion store the domain in
landlock_file(file)->fown_domain; control_current_fowner() is identical
there, so the same exemption and the same fix apply.

Fixes: 18eb75f3af40 ("landlock: Always allow signals between threads of the same process")
Cc: stable@vger.kernel.org
Reported-by: Bryam Vargas <hexlabsecurity@proton.me>
Tested-by: Justin Suess <utilityemal77@gmail.com>
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 security/landlock/fs.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index c1ecfe239032..edaa52572cbd 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1909,6 +1909,18 @@ static bool control_current_fowner(struct fown_struct *const fown)
 	if (!p)
 		return true;
 
+	/*
+	 * For PIDTYPE_PGID and PIDTYPE_SID, signal delivery fans out to
+	 * every member of the group at SIGIO time. Even when pid_task()
+	 * resolves to current itself (e.g., current is the pgid hlist
+	 * head post-fork), non-current members of the group are still
+	 * valid targets that must be checked by hook_file_send_sigiotask().
+	 * Always capture the current subject for those types so the
+	 * consumer scope check runs against the live fown_subject.
+	 */
+	if (fown->pid_type == PIDTYPE_PGID || fown->pid_type == PIDTYPE_SID)
+		return true;
+
 	return !same_thread_group(p, current);
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass via F_SETOWN to invoker's pgid
  2026-05-29 19:07 [PATCH v3 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass via F_SETOWN to invoker's pgid hexlabsecurity
@ 2026-05-31 10:41 ` Mickaël Salaün
  2026-06-02 17:27   ` [PATCH v4 0/2] landlock: fix SCOPE_SIGNAL bypass on the SIGIO/fowner path Bryam Vargas
  2026-06-01 22:08 ` [PATCH v3 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass via F_SETOWN to invoker's pgid Günther Noack
  1 sibling, 1 reply; 7+ messages in thread
From: Mickaël Salaün @ 2026-05-31 10:41 UTC (permalink / raw)
  To: hexlabsecurity
  Cc: Justin Suess, gnoack@google.com,
	linux-security-module@vger.kernel.org, stable@vger.kernel.org,
	Christian Brauner

On Fri, May 29, 2026 at 07:07:30PM +0000, hexlabsecurity@proton.me wrote:
> From b5fdc79ce1cb2881d59dfed01d3d9170306be9e8 Mon Sep 17 00:00:00 2001
> From: Bryam Vargas <hexlabsecurity@proton.me>
> Date: Fri, 29 May 2026 12:49:41 -0500
> Subject: [PATCH v3 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass via
>  F_SETOWN to invoker's pgid

Please send proper threaded emails.  Sending two unrelated
emails/patches and with additional headers in the body makes it more
difficult to handle and ignored by reviewer bots.

> 
> A Landlock-restricted process can bypass LANDLOCK_SCOPE_SIGNAL on the
> SIGIO delivery path and deliver arbitrary signals (including SIGKILL via
> F_SETSIG) to non-Landlocked targets that share its pgid, by exploiting a
> producer-side cache-vs-live evaluation gap.

What does mean: "producer-side cache-vs-live evaluation gap"?

It looks like the "cache" here is the fowner data, which is not a cache.

Could you please make it clear in the commit message what is the threat
(e.g. real-life scenario of a signal control bypass)?

> 
> The SIGIO path in hook_file_send_sigiotask() consults a cached subject
> stored in landlock_file(file)->fown_subject at fcntl(F_SETOWN) time
> (via hook_file_set_fowner()), instead of evaluating the live Landlock
> domain of the invoking task at signal-send time. The capture is gated
> by control_current_fowner(), which returns false (skipping capture)
> when pid_task(fown->pid, fown->pid_type) is in current's thread group.

Commit messages should mostly explain the "why", and only the minimal
"what".

A simpler commit message would be useful.

> 
> This is correct for PIDTYPE_TGID / PIDTYPE_PID, where the target is a
> single task sharing current's cred. It is unsafe for PIDTYPE_PGID and

By "task" do you mean "process"?

> PIDTYPE_SID: when current is at the head of its pgid hlist -- the
> default placement after fork(), hlist_add_head_rcu() in kernel/fork.c --
> pid_task(pgid, PIDTYPE_PGID) resolves to current itself,
> same_thread_group(current, current) is true, the capture is skipped, and
> fown_subject.domain stays NULL. hook_file_send_sigiotask() then
> short-circuits at "if (!subject->domain) return 0;", letting the kernel
> fan the signal out to every member of the group, including tasks outside
> current's Landlock domain that SCOPE_SIGNAL is supposed to protect.
> 
> The direct kill() path (hook_task_kill) is unaffected: it evaluates
> current's live domain on every call. Only the cached SIGIO path is
> broken.
> 
> Tighten control_current_fowner() to apply the thread-group exemption
> only when the target identifies a single task whose Landlock cred is
> necessarily shared with current (PIDTYPE_TGID, PIDTYPE_PID). For
> PIDTYPE_PGID and PIDTYPE_SID, always capture the current Landlock
> subject so the consumer's scope check runs against every member of the
> group at delivery time.

PIDTYPE_SID doesn't seem possible.

> 
> Stable kernels before the fown_subject conversion store the domain in
> landlock_file(file)->fown_domain; control_current_fowner() is identical
> there, so the same exemption and the same fix apply.
> 
> Fixes: 18eb75f3af40 ("landlock: Always allow signals between threads of the same process")
> Cc: stable@vger.kernel.org
> Reported-by: Bryam Vargas <hexlabsecurity@proton.me>

You should remove Reported-by because it is implicit when you also add a
Signed-off-by with the same value.

> Tested-by: Justin Suess <utilityemal77@gmail.com>
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
>  security/landlock/fs.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index c1ecfe239032..edaa52572cbd 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1909,6 +1909,18 @@ static bool control_current_fowner(struct fown_struct *const fown)
>  	if (!p)
>  		return true;
>  
> +	/*
> +	 * For PIDTYPE_PGID and PIDTYPE_SID, signal delivery fans out to
> +	 * every member of the group at SIGIO time. Even when pid_task()
> +	 * resolves to current itself (e.g., current is the pgid hlist
> +	 * head post-fork), non-current members of the group are still
> +	 * valid targets that must be checked by hook_file_send_sigiotask().
> +	 * Always capture the current subject for those types so the
> +	 * consumer scope check runs against the live fown_subject.

This looks good but could be simplified.

> +	 */
> +	if (fown->pid_type == PIDTYPE_PGID || fown->pid_type == PIDTYPE_SID)

PIDTYPE_SID is not possible for fowner, and I'd prefer a defensive
programming approach:

  fown->pid_type != PIDTYPE_PID && fown->pid_type != PIDTYPE_TGID

> +		return true;
> +
>  	return !same_thread_group(p, current);
>  }
>  
> -- 
> 2.43.0
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass via F_SETOWN to invoker's pgid
  2026-05-29 19:07 [PATCH v3 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass via F_SETOWN to invoker's pgid hexlabsecurity
  2026-05-31 10:41 ` Mickaël Salaün
@ 2026-06-01 22:08 ` Günther Noack
  1 sibling, 0 replies; 7+ messages in thread
From: Günther Noack @ 2026-06-01 22:08 UTC (permalink / raw)
  To: hexlabsecurity
  Cc: Mickaël Salaün, Justin Suess, gnoack@google.com,
	linux-security-module@vger.kernel.org, stable@vger.kernel.org

On Fri, May 29, 2026 at 07:07:30PM +0000, hexlabsecurity@proton.me wrote:
> From b5fdc79ce1cb2881d59dfed01d3d9170306be9e8 Mon Sep 17 00:00:00 2001
> From: Bryam Vargas <hexlabsecurity@proton.me>
> Date: Fri, 29 May 2026 12:49:41 -0500
> Subject: [PATCH v3 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass via
>  F_SETOWN to invoker's pgid
> 
> A Landlock-restricted process can bypass LANDLOCK_SCOPE_SIGNAL on the
> SIGIO delivery path and deliver arbitrary signals (including SIGKILL via
> F_SETSIG) to non-Landlocked targets that share its pgid, by exploiting a
> producer-side cache-vs-live evaluation gap.
> 
> The SIGIO path in hook_file_send_sigiotask() consults a cached subject
> stored in landlock_file(file)->fown_subject at fcntl(F_SETOWN) time
> (via hook_file_set_fowner()), instead of evaluating the live Landlock
> domain of the invoking task at signal-send time. The capture is gated
> by control_current_fowner(), which returns false (skipping capture)
> when pid_task(fown->pid, fown->pid_type) is in current's thread group.
> 
> This is correct for PIDTYPE_TGID / PIDTYPE_PID, where the target is a
> single task sharing current's cred. It is unsafe for PIDTYPE_PGID and
> PIDTYPE_SID: when current is at the head of its pgid hlist -- the
> default placement after fork(), hlist_add_head_rcu() in kernel/fork.c --
> pid_task(pgid, PIDTYPE_PGID) resolves to current itself,
> same_thread_group(current, current) is true, the capture is skipped, and
> fown_subject.domain stays NULL. hook_file_send_sigiotask() then
> short-circuits at "if (!subject->domain) return 0;", letting the kernel
> fan the signal out to every member of the group, including tasks outside
> current's Landlock domain that SCOPE_SIGNAL is supposed to protect.
> 
> The direct kill() path (hook_task_kill) is unaffected: it evaluates
> current's live domain on every call. Only the cached SIGIO path is
> broken.
> 
> Tighten control_current_fowner() to apply the thread-group exemption
> only when the target identifies a single task whose Landlock cred is
> necessarily shared with current (PIDTYPE_TGID, PIDTYPE_PID). For
> PIDTYPE_PGID and PIDTYPE_SID, always capture the current Landlock
> subject so the consumer's scope check runs against every member of the
> group at delivery time.
> 
> Stable kernels before the fown_subject conversion store the domain in
> landlock_file(file)->fown_domain; control_current_fowner() is identical
> there, so the same exemption and the same fix apply.
> 
> Fixes: 18eb75f3af40 ("landlock: Always allow signals between threads of the same process")
> Cc: stable@vger.kernel.org
> Reported-by: Bryam Vargas <hexlabsecurity@proton.me>
> Tested-by: Justin Suess <utilityemal77@gmail.com>
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
>  security/landlock/fs.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index c1ecfe239032..edaa52572cbd 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1909,6 +1909,18 @@ static bool control_current_fowner(struct fown_struct *const fown)
>  	if (!p)
>  		return true;
>  
> +	/*
> +	 * For PIDTYPE_PGID and PIDTYPE_SID, signal delivery fans out to
> +	 * every member of the group at SIGIO time. Even when pid_task()
> +	 * resolves to current itself (e.g., current is the pgid hlist
> +	 * head post-fork), non-current members of the group are still
> +	 * valid targets that must be checked by hook_file_send_sigiotask().
> +	 * Always capture the current subject for those types so the
> +	 * consumer scope check runs against the live fown_subject.
> +	 */
> +	if (fown->pid_type == PIDTYPE_PGID || fown->pid_type == PIDTYPE_SID)
> +		return true;
> +
>  	return !same_thread_group(p, current);
>  }

The reason why the same_thread_group() check exists is so that Go
programs that had to use libpsx instead of TSYNC had a way to signal
their own OS threads at the C level (a feature used by linked C
libraries and specifically by libpsx itself, so it prevented nested
Landlock domains).

(a) On Linux 7.0, the Go-Landlock library automatically uses TSYNC so
    this is not a problem any more.

(b) On earlier Linux versions

    * libpsx signaling is also going to continue working,
      because it uses normal signals instead of SIGIO

    * other libraries are also likely to continue working, unless they
      use the somewhat obscure SIGIO with PIDTYPE_PGID or PIDTYPE_SID.

    There is little incentive to use SIGIO in a pure Go program, as
    the runtime already implements file descriptor polling logic (with
    epoll, which is anyway a better choice)

So, this looks fine from the Go perspective; I doubt that this has
practical implications for Go.

Thank you for spotting this and providing a fix! 🙏

–Günther

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 0/2] landlock: fix SCOPE_SIGNAL bypass on the SIGIO/fowner path
  2026-05-31 10:41 ` Mickaël Salaün
@ 2026-06-02 17:27   ` Bryam Vargas
       [not found]     ` <20260602172741.18760-2-hexlabsecurity@proton.me>
  0 siblings, 1 reply; 7+ messages in thread
From: Bryam Vargas @ 2026-06-02 17:27 UTC (permalink / raw)
  To: Mickaël Salaün, Günther Noack
  Cc: Justin Suess, Christian Brauner, Paul Moore, James Morris,
	Serge E . Hallyn, linux-security-module, stable, linux-kernel

This series fixes a LANDLOCK_SCOPE_SIGNAL bypass on the asynchronous SIGIO
(fcntl(F_SETOWN)) delivery path, and adds a regression test.

A sandboxed process that owns a file or socket can request a signal
(F_SETSIG, e.g. SIGKILL) to be delivered to a whole process group on I/O
readiness (F_SETOWN(-pgid) + O_ASYNC).  When it is the head of its own
process group -- the default after fork() -- that group still contains the
non-sandboxed process that launched it (a supervisor, a security monitor),
so the sandbox can signal processes that SCOPE_SIGNAL is meant to protect
from it.

Patch 1 narrows the same-thread-group exemption in control_current_fowner()
so a process-group fowner always records the caller's Landlock domain; the
delivery-time check in hook_file_send_sigiotask() then runs against every
group member.  The direct kill() path (hook_task_kill) is unaffected.

Patch 2 adds the regression test in scoped_signal_test.c.

The defect was introduced by commit 18eb75f3af40 ("landlock: Always allow
signals between threads of the same process") in v6.15, and is present in the
stable branches that backported it (6.12.y, 6.13.y, 6.14.y).
control_current_fowner() is identical across those branches.

A/B verified on 6.12.90 + CONFIG_SECURITY_LANDLOCK (same .config, only the fix
hunk differs): without patch 1 the new test fails (the non-sandboxed parent is
signaled, SCOPE_SIGNAL bypassed); with patch 1 the new test passes and the
landlock signal-scoping suite is 20/20.

v3 -> v4 (review feedback from Mickaël Salaün):
  - patch 1: rewrite the commit message -- drop the "cache" framing, lead with
    the threat scenario, mostly "why" and minimal "what", "process" not "task";
  - patch 1: drop PIDTYPE_SID (not possible for an fowner) and use the defensive
    condition "!= PIDTYPE_PID && != PIDTYPE_TGID"; simplify the in-code comment;
  - patch 1: remove Reported-by (implicit with the same Signed-off-by);
  - send as a proper git send-email threaded series.
  - v1/v2 were sent to security@kernel.org (embargoed; not in a public archive).

Bryam Vargas (2):
  landlock: fix LANDLOCK_SCOPE_SIGNAL bypass on the SIGIO path
  selftests/landlock: test SCOPE_SIGNAL on the SIGIO/fowner pgid path

 security/landlock/fs.c                        |  9 ++
 .../selftests/landlock/scoped_signal_test.c   | 97 +++++++++++++++++++
 2 files changed, 106 insertions(+)


base-commit: 6f3ed7fec72fc8979b2a8c7219c0a9fcfc8d07b5
-- 
2.43.0



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass on the SIGIO path
       [not found]     ` <20260602172741.18760-2-hexlabsecurity@proton.me>
@ 2026-06-04  8:10       ` Günther Noack
  2026-06-04 10:27         ` Bryam Vargas
  0 siblings, 1 reply; 7+ messages in thread
From: Günther Noack @ 2026-06-04  8:10 UTC (permalink / raw)
  To: Bryam Vargas
  Cc: Mickaël Salaün, Günther Noack, Justin Suess,
	Christian Brauner, Paul Moore, James Morris, Serge E . Hallyn,
	linux-security-module, stable, linux-kernel

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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass on the SIGIO path
  2026-06-04  8:10       ` [PATCH v4 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass on the SIGIO path Günther Noack
@ 2026-06-04 10:27         ` Bryam Vargas
  2026-06-04 20:47           ` Günther Noack
  0 siblings, 1 reply; 7+ messages in thread
From: Bryam Vargas @ 2026-06-04 10:27 UTC (permalink / raw)
  To: Günther Noack
  Cc: Mickaël Salaün, Günther Noack, Justin Suess,
	Christian Brauner, Paul Moore, James Morris, Serge E . Hallyn,
	linux-security-module, stable, linux-kernel

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass on the SIGIO path
  2026-06-04 10:27         ` Bryam Vargas
@ 2026-06-04 20:47           ` Günther Noack
  0 siblings, 0 replies; 7+ messages in thread
From: Günther Noack @ 2026-06-04 20:47 UTC (permalink / raw)
  To: Bryam Vargas
  Cc: Mickaël Salaün, Günther Noack, Justin Suess,
	Christian Brauner, Paul Moore, James Morris, Serge E . Hallyn,
	linux-security-module, stable, linux-kernel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-06-04 20:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-29 19:07 [PATCH v3 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass via F_SETOWN to invoker's pgid hexlabsecurity
2026-05-31 10:41 ` Mickaël Salaün
2026-06-02 17:27   ` [PATCH v4 0/2] landlock: fix SCOPE_SIGNAL bypass on the SIGIO/fowner path Bryam Vargas
     [not found]     ` <20260602172741.18760-2-hexlabsecurity@proton.me>
2026-06-04  8:10       ` [PATCH v4 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass on the SIGIO path Günther Noack
2026-06-04 10:27         ` Bryam Vargas
2026-06-04 20:47           ` Günther Noack
2026-06-01 22:08 ` [PATCH v3 1/2] landlock: fix LANDLOCK_SCOPE_SIGNAL bypass via F_SETOWN to invoker's pgid Günther Noack

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox