* [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
* [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
[parent not found: <20260602172741.18760-2-hexlabsecurity@proton.me>]
* 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
* 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
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