* [PATCH] landlock: Move the domain layer counter out of the anonymous union
@ 2026-10-07 16:28 Abel Vesa
2026-10-07 21:49 ` Nick Desaulniers
2026-10-09 11:51 ` Mike Bommarito
0 siblings, 2 replies; 8+ messages in thread
From: Abel Vesa @ 2026-10-07 16:28 UTC (permalink / raw)
To: Mickaël Salaün, Günther Noack, Paul Moore,
James Morris, Serge E. Hallyn, Kees Cook, Gustavo A. R. Silva,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Tingmao Wang
Cc: linux-security-module, linux-kernel, linux-hardening, llvm,
Konrad Dybcio, Abel Vesa
With Clang 20 and CONFIG_FORTIFY_SOURCE, stacking Landlock domains can
trigger a fortify panic in the handled_masks copy in inherit_ruleset():
__fortify_panic
landlock_merge_ruleset
__arm64_sys_landlock_restrict_self
Clang miscalculates the location of the __counted_by counter in the
anonymous structure nested inside the domain's union. In an arm64 build,
__builtin_dynamic_object_size(domain->handled_masks, 1) reads offset 40
(the first handled_masks entry) instead of offset 36 (num_layers).
Because the destination domain is zero-initialized and its masks have not
yet been populated, FORTIFY sees a zero-sized destination and rejects the
copy of the parent's layers.
Move num_layers and handled_masks to the top level of landlock_domain so
that Clang uses the correct counter.
Assisted-by: LLM
Fixes: bd3a19800dd1 ("landlock: Add counted_by in landlock_domain")
Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Co-developed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
Move the domain layer counter and flexible array out of the anonymous
union to work around Clang 20 reading the wrong counter for __counted_by.
This fixes the FORTIFY panic when copying the parent domain's layer stack.
Based on next-20260930. Validated arm64 Landlock object builds with
Clang 20 and GCC 14, with CONFIG_FORTIFY_SOURCE enabled, and checked the
counter load in Clang-generated code. Not boot-tested.
---
security/landlock/domain.h | 48 +++++++++++++++++++---------------------------
1 file changed, 20 insertions(+), 28 deletions(-)
diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index caa3d19d2c43..5e65ab004f62 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -216,37 +216,29 @@ struct landlock_domain {
/**
* @work_free: Enables to free a domain within a lockless
* section. This is only used by landlock_put_domain_deferred()
- * when @usage reaches zero. The fields @usage, @num_layers and
- * @handled_masks are then unused.
+ * when @usage reaches zero. The field @usage is then unused.
*/
struct work_struct work_free;
- struct {
- /**
- * @usage: Number of credentials referencing this
- * domain.
- */
- refcount_t usage;
- /**
- * @num_layers: Number of layers that are used in this
- * domain. This enables to check that all the layers
- * allow an access request.
- */
- u32 num_layers;
- /**
- * @handled_masks: Contains the subset of filesystem and
- * network actions that are restricted by a domain. A
- * domain saves all layers of merged rulesets in a stack
- * (FAM), starting from the first layer to the last one.
- * These layers are used when merging rulesets, for user
- * space backward compatibility (i.e. future-proof), and
- * to properly handle merged rulesets without
- * overlapping access rights. These layers are set once
- * and never changed for the lifetime of the domain.
- */
- struct access_masks
- handled_masks[] __counted_by(num_layers);
- };
+ /**
+ * @usage: Number of credentials referencing this domain.
+ */
+ refcount_t usage;
};
+ /**
+ * @num_layers: Number of layers that are used in this domain. This
+ * enables to check that all the layers allow an access request.
+ */
+ u32 num_layers;
+ /**
+ * @handled_masks: Contains the subset of filesystem and network actions
+ * that are restricted by a domain. A domain saves all layers of merged
+ * rulesets in a stack (FAM), starting from the first layer to the last
+ * one. These layers are used when merging rulesets, for user space
+ * backward compatibility (i.e. future-proof), and to properly handle
+ * merged rulesets without overlapping access rights. These layers are
+ * set once and never changed for the lifetime of the domain.
+ */
+ struct access_masks handled_masks[] __counted_by(num_layers);
};
static inline access_mask_t
---
base-commit: 6c2cb8b8b843d216ab549b678a0d8831c43153e0
change-id: 20261007-b4-landlock-fix-domain-fortify-panic-510367f8cfe3
Best regards,
--
Abel Vesa <abel.vesa@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] landlock: Move the domain layer counter out of the anonymous union 2026-10-07 16:28 [PATCH] landlock: Move the domain layer counter out of the anonymous union Abel Vesa @ 2026-10-07 21:49 ` Nick Desaulniers 2026-10-08 11:49 ` Abel Vesa 2026-10-09 11:51 ` Mike Bommarito 1 sibling, 1 reply; 8+ messages in thread From: Nick Desaulniers @ 2026-10-07 21:49 UTC (permalink / raw) To: Abel Vesa, Bill Wendling, Tingmao Wang, Nathan Chancellor, Kees Cook Cc: Gustavo A. R. Silva, Justin Stitt, linux-security-module, linux-kernel, linux-hardening, llvm, Konrad Dybcio On Wed, Oct 7, 2026 at 9:28 AM Abel Vesa <abel.vesa@oss.qualcomm.com> wrote: > > With Clang 20 and CONFIG_FORTIFY_SOURCE, stacking Landlock domains can > trigger a fortify panic in the handled_masks copy in inherit_ruleset(): > > __fortify_panic > landlock_merge_ruleset > __arm64_sys_landlock_restrict_self > > Clang miscalculates the location of the __counted_by counter in the Thanks for the patch. Would you consider this a bug in clang, or an error in how the __counted_by attribute is applied? If the former, then a bug report against upstream llvm-project/ and a link to that bug report would be appreciated. Then Bill can take a look. Or if this was a known issue specific to clang 20, then we should have a link to the fix (or consider bumping counted_by support to the compiler version that is bug free). > anonymous structure nested inside the domain's union. In an arm64 build, > __builtin_dynamic_object_size(domain->handled_masks, 1) reads offset 40 > (the first handled_masks entry) instead of offset 36 (num_layers). > Because the destination domain is zero-initialized and its masks have not > yet been populated, FORTIFY sees a zero-sized destination and rejects the > copy of the parent's layers. > > Move num_layers and handled_masks to the top level of landlock_domain so > that Clang uses the correct counter. > > Assisted-by: LLM > Fixes: bd3a19800dd1 ("landlock: Add counted_by in landlock_domain") > Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > Co-developed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> > Signed-off-by: Abel Vesa <abel.vesa@oss.qualcomm.com> > --- > Move the domain layer counter and flexible array out of the anonymous > union to work around Clang 20 reading the wrong counter for __counted_by. > This fixes the FORTIFY panic when copying the parent domain's layer stack. > > Based on next-20260930. Validated arm64 Landlock object builds with > Clang 20 and GCC 14, with CONFIG_FORTIFY_SOURCE enabled, and checked the > counter load in Clang-generated code. Not boot-tested. > --- > security/landlock/domain.h | 48 +++++++++++++++++++--------------------------- > 1 file changed, 20 insertions(+), 28 deletions(-) > > diff --git a/security/landlock/domain.h b/security/landlock/domain.h > index caa3d19d2c43..5e65ab004f62 100644 > --- a/security/landlock/domain.h > +++ b/security/landlock/domain.h > @@ -216,37 +216,29 @@ struct landlock_domain { > /** > * @work_free: Enables to free a domain within a lockless > * section. This is only used by landlock_put_domain_deferred() > - * when @usage reaches zero. The fields @usage, @num_layers and > - * @handled_masks are then unused. > + * when @usage reaches zero. The field @usage is then unused. > */ > struct work_struct work_free; > - struct { > - /** > - * @usage: Number of credentials referencing this > - * domain. > - */ > - refcount_t usage; > - /** > - * @num_layers: Number of layers that are used in this > - * domain. This enables to check that all the layers > - * allow an access request. > - */ > - u32 num_layers; > - /** > - * @handled_masks: Contains the subset of filesystem and > - * network actions that are restricted by a domain. A > - * domain saves all layers of merged rulesets in a stack > - * (FAM), starting from the first layer to the last one. > - * These layers are used when merging rulesets, for user > - * space backward compatibility (i.e. future-proof), and > - * to properly handle merged rulesets without > - * overlapping access rights. These layers are set once > - * and never changed for the lifetime of the domain. > - */ > - struct access_masks > - handled_masks[] __counted_by(num_layers); > - }; > + /** > + * @usage: Number of credentials referencing this domain. > + */ > + refcount_t usage; > }; > + /** > + * @num_layers: Number of layers that are used in this domain. This > + * enables to check that all the layers allow an access request. > + */ > + u32 num_layers; > + /** > + * @handled_masks: Contains the subset of filesystem and network actions > + * that are restricted by a domain. A domain saves all layers of merged > + * rulesets in a stack (FAM), starting from the first layer to the last > + * one. These layers are used when merging rulesets, for user space > + * backward compatibility (i.e. future-proof), and to properly handle > + * merged rulesets without overlapping access rights. These layers are > + * set once and never changed for the lifetime of the domain. > + */ > + struct access_masks handled_masks[] __counted_by(num_layers); > }; > > static inline access_mask_t > > --- > base-commit: 6c2cb8b8b843d216ab549b678a0d8831c43153e0 > change-id: 20261007-b4-landlock-fix-domain-fortify-panic-510367f8cfe3 > > Best regards, > -- > Abel Vesa <abel.vesa@oss.qualcomm.com> > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] landlock: Move the domain layer counter out of the anonymous union 2026-10-07 21:49 ` Nick Desaulniers @ 2026-10-08 11:49 ` Abel Vesa 2026-10-09 17:23 ` Nick Desaulniers 0 siblings, 1 reply; 8+ messages in thread From: Abel Vesa @ 2026-10-08 11:49 UTC (permalink / raw) To: Nick Desaulniers Cc: Bill Wendling, Tingmao Wang, Nathan Chancellor, Kees Cook, Gustavo A. R. Silva, Justin Stitt, linux-security-module, linux-kernel, linux-hardening, llvm, Konrad Dybcio On 26-10-07 14:49:28, Nick Desaulniers wrote: > On Wed, Oct 7, 2026 at 9:28 AM Abel Vesa <abel.vesa@oss.qualcomm.com> wrote: > > > > With Clang 20 and CONFIG_FORTIFY_SOURCE, stacking Landlock domains can > > trigger a fortify panic in the handled_masks copy in inherit_ruleset(): > > > > __fortify_panic > > landlock_merge_ruleset > > __arm64_sys_landlock_restrict_self > > > > Clang miscalculates the location of the __counted_by counter in the > > Thanks for the patch. > > Would you consider this a bug in clang, or an error in how the > __counted_by attribute is applied? > > If the former, then a bug report against upstream llvm-project/ and a > link to that bug report would be appreciated. Then Bill can take a > look. > > Or if this was a known issue specific to clang 20, then we should have > a link to the fix (or consider bumping counted_by support to the > compiler version that is bug free). This is a Clang code generation bug, and it is not limited to Clang 20. I checked a reduced reproducer with Clang 20.1.2, 21.1.8, 22.1.8 and 23.1.2. All four reproduce it at both -O0 and -O2. The native x86_64 runtime test returns zero for the dynamic object size of the zero-initialized array, despite num_layers being set. The AArch64 assembly also shows the count being loaded from offset 40 rather than num_layers at offset 36. Moving the counter and array out of the union makes the reproducer pass with all four versions. It turns out Amaan Qureshi has already reported exactly this Landlock issue and proposed an LLVM fix: https://github.com/llvm/llvm-project/pull/228309 Thanks, Abel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] landlock: Move the domain layer counter out of the anonymous union 2026-10-08 11:49 ` Abel Vesa @ 2026-10-09 17:23 ` Nick Desaulniers 2026-10-09 20:45 ` Nathan Chancellor 0 siblings, 1 reply; 8+ messages in thread From: Nick Desaulniers @ 2026-10-09 17:23 UTC (permalink / raw) To: Abel Vesa, Bill Wendling, Nathan Chancellor, Kees Cook Cc: Tingmao Wang, Gustavo A. R. Silva, Justin Stitt, linux-security-module, linux-kernel, linux-hardening, llvm, Konrad Dybcio, michael.bommarito On Thu, Oct 8, 2026 at 4:49 AM Abel Vesa <abel.vesa@oss.qualcomm.com> wrote: > > On 26-10-07 14:49:28, Nick Desaulniers wrote: > > Would you consider this a bug in clang, or an error in how the > > __counted_by attribute is applied? > > > It turns out Amaan Qureshi has already reported exactly this Landlock > issue and proposed an LLVM fix: > > https://github.com/llvm/llvm-project/pull/228309 In that case, I would _not_ work around the compiler bug like this and instead wait for that fix to land in clang, then bump the required version of clang for counted-by. Kees and Nathan are traveling for LPC, so I don't expect quick feedback from them, but they should confirm whether they agree with my proposal. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] landlock: Move the domain layer counter out of the anonymous union 2026-10-09 17:23 ` Nick Desaulniers @ 2026-10-09 20:45 ` Nathan Chancellor 2026-10-09 23:52 ` Kees Cook 0 siblings, 1 reply; 8+ messages in thread From: Nathan Chancellor @ 2026-10-09 20:45 UTC (permalink / raw) To: Nick Desaulniers Cc: Abel Vesa, Bill Wendling, Kees Cook, Tingmao Wang, Gustavo A. R. Silva, Justin Stitt, linux-security-module, linux-kernel, linux-hardening, llvm, Konrad Dybcio, michael.bommarito On Fri, Oct 09, 2026 at 10:23:14AM -0700, Nick Desaulniers wrote: > On Thu, Oct 8, 2026 at 4:49 AM Abel Vesa <abel.vesa@oss.qualcomm.com> wrote: > > > > On 26-10-07 14:49:28, Nick Desaulniers wrote: > > > Would you consider this a bug in clang, or an error in how the > > > __counted_by attribute is applied? > > > > > It turns out Amaan Qureshi has already reported exactly this Landlock > > issue and proposed an LLVM fix: > > > > https://github.com/llvm/llvm-project/pull/228309 > > In that case, I would _not_ work around the compiler bug like this and > instead wait for that fix to land in clang, then bump the required > version of clang for counted-by. > > Kees and Nathan are traveling for LPC, so I don't expect quick > feedback from them, but they should confirm whether they agree with my > proposal. Bill/Kees/Justin should review that patch upstream but bumping the required version for __counted_by to clang-24 (or maybe 23.1.x if it can land in release/23.x) kind of sucks :/ but if this is a big enough footgun that we don't want to continuously workaround, I guess we have no choice. -- Cheers, Nathan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] landlock: Move the domain layer counter out of the anonymous union 2026-10-09 20:45 ` Nathan Chancellor @ 2026-10-09 23:52 ` Kees Cook 2026-10-10 0:02 ` Michael Bommarito 0 siblings, 1 reply; 8+ messages in thread From: Kees Cook @ 2026-10-09 23:52 UTC (permalink / raw) To: Nathan Chancellor, Nick Desaulniers Cc: Abel Vesa, Bill Wendling, Tingmao Wang, Gustavo A. R. Silva, Justin Stitt, linux-security-module, linux-kernel, linux-hardening, llvm, Konrad Dybcio, michael.bommarito On October 9, 2026 1:45:54 PM PDT, Nathan Chancellor <nathan@kernel.org> wrote: >On Fri, Oct 09, 2026 at 10:23:14AM -0700, Nick Desaulniers wrote: >> On Thu, Oct 8, 2026 at 4:49 AM Abel Vesa <abel.vesa@oss.qualcomm.com> wrote: >> > >> > On 26-10-07 14:49:28, Nick Desaulniers wrote: >> > > Would you consider this a bug in clang, or an error in how the >> > > __counted_by attribute is applied? >> > > >> > It turns out Amaan Qureshi has already reported exactly this Landlock >> > issue and proposed an LLVM fix: >> > >> > https://github.com/llvm/llvm-project/pull/228309 >> >> In that case, I would _not_ work around the compiler bug like this and >> instead wait for that fix to land in clang, then bump the required >> version of clang for counted-by. >> >> Kees and Nathan are traveling for LPC, so I don't expect quick >> feedback from them, but they should confirm whether they agree with my >> proposal. > >Bill/Kees/Justin should review that patch upstream but bumping the >required version for __counted_by to clang-24 (or maybe 23.1.x if it can >land in release/23.x) kind of sucks :/ but if this is a big enough >footgun that we don't want to continuously workaround, I guess we have >no choice. I'm still catching up from travel, so apologies if this already got checked, but has this been verified against GCC as well? Is it only a Clang problem? Regardless, if counted_by is not working for a given compiler version we'll need to exclude its use by version. :( -Kees -- Kees Cook ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] landlock: Move the domain layer counter out of the anonymous union 2026-10-09 23:52 ` Kees Cook @ 2026-10-10 0:02 ` Michael Bommarito 0 siblings, 0 replies; 8+ messages in thread From: Michael Bommarito @ 2026-10-10 0:02 UTC (permalink / raw) To: Kees Cook Cc: Nathan Chancellor, Nick Desaulniers, Abel Vesa, Bill Wendling, Tingmao Wang, Gustavo A. R. Silva, Justin Stitt, linux-security-module, linux-kernel, linux-hardening, llvm, Konrad Dybcio On Fri, Oct 9, 2026 at 7:52 PM Kees Cook <kees@kernel.org> wrote: > I'm still catching up from travel, so apologies if this already got checked, but has this been verified against GCC as well? Is it only a Clang problem? > > Regardless, if counted_by is not working for a given compiler version we'll need to exclude its use by version. :( It is only clang, not gcc. See my comparison between gcc 15 and clang 21.1.8 output in [1]. I didn't test clang 20 or 23 like Abel did, but it seems like we can't gate on any available clang version. [1] https://github.com/llvm/llvm-project/pull/228309#issuecomment-6080475086 Thanks, Mike ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] landlock: Move the domain layer counter out of the anonymous union 2026-10-07 16:28 [PATCH] landlock: Move the domain layer counter out of the anonymous union Abel Vesa 2026-10-07 21:49 ` Nick Desaulniers @ 2026-10-09 11:51 ` Mike Bommarito 1 sibling, 0 replies; 8+ messages in thread From: Mike Bommarito @ 2026-10-09 11:51 UTC (permalink / raw) To: Abel Vesa Cc: Mickaël Salaün, Günther Noack, Paul Moore, James Morris, Serge E . Hallyn, Kees Cook, Gustavo A . R . Silva, Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt, Tingmao Wang, Konrad Dybcio, linux-security-module, linux-kernel, linux-hardening, llvm On Wed, Oct 07, 2026 at 07:28:30PM +0300, Abel Vesa wrote: > With Clang 20 and CONFIG_FORTIFY_SOURCE, stacking Landlock domains can > trigger a fortify panic in the handled_masks copy in inherit_ruleset(): [...] > Based on next-20260930. Validated arm64 Landlock object builds with > Clang 20 and GCC 14, with CONFIG_FORTIFY_SOURCE enabled, and checked the > counter load in Clang-generated code. Not boot-tested. Hi all, hit this issue yesterday on x86_64 (Framework 13, Meteor Lake), v7.3-rc5 built with clang 21.1.8 and CONFIG_FORTIFY_SOURCE=y. I triggered it simply by opening the printer settings on 26.04 (through xz, see below). My build tree was recent v7.3-rc5, so I applied the patch, which was clean, and then went through the repro steps after rebuilding. Everything works as expected after 10-15 minutes of use - no crash, no other dmesg/journal issues. I hope you don't mind, but given how irritating it was to track down the issue, I am including some of the Claude output below to help others find the root cause more quickly if they hit it. I checked the disassembly and ran the tests below myself on this machine. <CLAUDE> This is easy to hit on a stock desktop. xz 5.8 sandboxes itself with nested landlock_restrict_self() calls, so on an unpatched clang build every `xz -d` oopses: memcpy: detected buffer overflow: 4 byte write of buffer size 0 WARNING: lib/string_helpers.c:1037 at __fortify_report+0x1f/0x60, CPU#6: xz/9888 kernel BUG at lib/string_helpers.c:1044! Oops: invalid opcode: 0000 [#1] SMP NOPTI RIP: 0010:__fortify_panic+0x9/0x10 Call Trace: landlock_merge_ruleset+0x353/0x360 __se_sys_landlock_restrict_self+0x1bf/0x470 __x64_sys_landlock_restrict_self+0x16/0x30 On Ubuntu, simply opening GNOME Settings -> Printers runs the CUPS driver helpers, which run xz. With kdump-tools installed (panic_on_oops=1), that panics the machine. Nothing more than an unprivileged process is needed; the reproducer below triggers it as a normal user: prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); for (i = 0; i < 2; i++) { struct landlock_ruleset_attr a = { .handled_access_fs = LANDLOCK_ACCESS_FS_MAKE_REG }; int fd = syscall(SYS_landlock_create_ruleset, &a, sizeof(a), 0); syscall(SYS_landlock_restrict_self, fd, 0); /* i == 1 oopses */ close(fd); } The x86_64 codegen matches your arm64 analysis. Unpatched, with num_layers at 0x24 and handled_masks at 0x28: mov 0x28(%rbx),%esi # "count" read from child->handled_masks[0] shl $0x2,%rsi cmp %rdx,%rsi jb <__fortify_panic> With the patch, num_layers is at 0x40 and handled_masks at 0x44. The count is loaded from the right field, and clang folds away the fortify checks after the WARN_ON_ONCE(child->num_layers <= parent->num_layers): mov 0x40(%r12),%edx cmp %edx,0x40(%rbx) ... call memcpy Results after booting the patched kernel, with panic_on_oops=1 and kdump loaded (the configuration that used to panic): nested restrict reproducer: both layers succeed echo hello | xz | xz -d: ok lpinfo -m (CUPS -> xz): ok fortify reports in dmesg: none; kernel not tainted The layout change grows struct landlock_domain from 64 to 72 bytes. A one-layer domain is still a kmalloc-96 allocation (68 bytes before, 76 after); only stacks of 7 or 8 layers move up to kmalloc-128. That seems fine. </CLAUDE> Since this is reachable from unprivileged userspace on any clang + FORTIFY_SOURCE build of 7.3-rc, it would be good to get this in before v7.3 final. Tested-by: Mike Bommarito <michael.bommarito@gmail.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-10 0:02 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-07 16:28 [PATCH] landlock: Move the domain layer counter out of the anonymous union Abel Vesa 2026-10-07 21:49 ` Nick Desaulniers 2026-10-08 11:49 ` Abel Vesa 2026-10-09 17:23 ` Nick Desaulniers 2026-10-09 20:45 ` Nathan Chancellor 2026-10-09 23:52 ` Kees Cook 2026-10-10 0:02 ` Michael Bommarito 2026-10-09 11:51 ` Mike Bommarito
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®