[PATCH] landlock: Drop __counted_by from landlock_domain.layers

[PATCH] landlock: Drop __counted_by from landlock_domain.layers

[Nguyen Ngoc Thang]

With clang 22 and CONFIG_FORTIFY_SOURCE, the second
landlock_restrict_self() call hits a bogus fortify report in
inherit_ruleset():

  memcpy: detected buffer overflow: 16 byte read of buffer size 0
  Call Trace:
   __fortify_panic
   landlock_merge_ruleset+0xc46/0xc90
   __se_sys_landlock_restrict_self+0x2bc/0xd40

layers[] lives in an anonymous struct inside the anonymous union
that overlays work_free.  For such a flexible array, clang computes the
address of the __counted_by counter as the start of the array instead of
&num_layers.  The bound is then read from layers[0], i.e. from
the first layer's configuration, which is not a count.  The same layout
crashes clang itself in a standalone test.

The annotation was added as documentation, so drop it rather than
restructure the union.  Keep the information in the member comment.

Reported-by: syzbot+a0eecb22de4d821a9e5d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a0eecb22de4d821a9e5d
Fixes: bd3a19800dd1 ("landlock: Add counted_by in landlock_domain")
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
Reproduced with clang 22.1.8 and FORTIFY_SOURCE on mic/next (64b9e6eff524):
the second landlock_restrict_self() hits the report, while 16 nested
restricts run clean with this patch.  Mickaël, if you prefer to just drop
bd3a19800dd1 from next, that works as well.
 security/landlock/domain.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index 60e8eb2d6ac2..4bf1555a4349 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -246,8 +246,12 @@ struct landlock_domain {
 			 * properly handle merged rulesets without overlapping
 			 * access rights.  These layers are set once and never
 			 * changed for the lifetime of the domain.
+			 *
+			 * Not annotated with __counted_by(@num_layers): clang
+			 * miscomputes the counter address for a FAM nested in an
+			 * anonymous struct within a union, which trips FORTIFY.
 			 */
-			struct layer_config layers[] __counted_by(num_layers);
+			struct layer_config layers[];
 		};
 	};
 };
-- 
2.43.0

Re: [PATCH] landlock: Drop __counted_by from landlock_domain.layers

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2747 bytes --]

Hello Nguyen,

> Date: 2026-10-10 23:44:49+0700
> From: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
>
> With clang 22 and CONFIG_FORTIFY_SOURCE, the second
> landlock_restrict_self() call hits a bogus fortify report in
> inherit_ruleset():
> 
>   memcpy: detected buffer overflow: 16 byte read of buffer size 0
>   Call Trace:
>    __fortify_panic
>    landlock_merge_ruleset+0xc46/0xc90
>    __se_sys_landlock_restrict_self+0x2bc/0xd40
> 
> layers[] lives in an anonymous struct inside the anonymous union
> that overlays work_free.  For such a flexible array, clang computes the
> address of the __counted_by counter as the start of the array instead of
> &num_layers.  The bound is then read from layers[0], i.e. from
> the first layer's configuration, which is not a count.  The same layout
> crashes clang itself in a standalone test.
> 
> The annotation was added as documentation, so drop it rather than
> restructure the union.  Keep the information in the member comment.
> 
> Reported-by: syzbot+a0eecb22de4d821a9e5d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a0eecb22de4d821a9e5d
> Fixes: bd3a19800dd1 ("landlock: Add counted_by in landlock_domain")
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
> Reproduced with clang 22.1.8 and FORTIFY_SOURCE on mic/next (64b9e6eff524):
> the second landlock_restrict_self() hits the report, while 16 nested
> restricts run clean with this patch.  Mickaël, if you prefer to just drop
> bd3a19800dd1 from next, that works as well.

Is this reproducible with Clang 23 or later?

Should __counted_by() expand to nothing for bogus versions of Clang?
That would allow using it, and just ignore Clang when it's broken,
rather than not using safety features.


Have a lovely day!
Alex

>  security/landlock/domain.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> index 60e8eb2d6ac2..4bf1555a4349 100644
> --- a/security/landlock/domain.h
> +++ b/security/landlock/domain.h
> @@ -246,8 +246,12 @@ struct landlock_domain {
>  			 * properly handle merged rulesets without overlapping
>  			 * access rights.  These layers are set once and never
>  			 * changed for the lifetime of the domain.
> +			 *
> +			 * Not annotated with __counted_by(@num_layers): clang
> +			 * miscomputes the counter address for a FAM nested in an
> +			 * anonymous struct within a union, which trips FORTIFY.
>  			 */
> -			struct layer_config layers[] __counted_by(num_layers);
> +			struct layer_config layers[];
>  		};
>  	};
>  };
> -- 
> 2.43.0
> 
> 

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] landlock: Drop __counted_by from landlock_domain.layers

[Nathan Chancellor]

On Sat, Oct 10, 2026 at 08:05:48PM +0200, Alejandro Colomar wrote:
> Hello Nguyen,
> 
> > Date: 2026-10-10 23:44:49+0700
> > From: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> >
> > With clang 22 and CONFIG_FORTIFY_SOURCE, the second
> > landlock_restrict_self() call hits a bogus fortify report in
> > inherit_ruleset():
> > 
> >   memcpy: detected buffer overflow: 16 byte read of buffer size 0
> >   Call Trace:
> >    __fortify_panic
> >    landlock_merge_ruleset+0xc46/0xc90
> >    __se_sys_landlock_restrict_self+0x2bc/0xd40
> > 
> > layers[] lives in an anonymous struct inside the anonymous union
> > that overlays work_free.  For such a flexible array, clang computes the
> > address of the __counted_by counter as the start of the array instead of
> > &num_layers.  The bound is then read from layers[0], i.e. from
> > the first layer's configuration, which is not a count.  The same layout
> > crashes clang itself in a standalone test.
> > 
> > The annotation was added as documentation, so drop it rather than
> > restructure the union.  Keep the information in the member comment.
> > 
> > Reported-by: syzbot+a0eecb22de4d821a9e5d@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=a0eecb22de4d821a9e5d
> > Fixes: bd3a19800dd1 ("landlock: Add counted_by in landlock_domain")
> > Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> > ---
> > Reproduced with clang 22.1.8 and FORTIFY_SOURCE on mic/next (64b9e6eff524):
> > the second landlock_restrict_self() hits the report, while 16 nested
> > restricts run clean with this patch.  Mickaël, if you prefer to just drop
> > bd3a19800dd1 from next, that works as well.
> 
> Is this reproducible with Clang 23 or later?
> 
> Should __counted_by() expand to nothing for bogus versions of Clang?
> That would allow using it, and just ignore Clang when it's broken,
> rather than not using safety features.

This is actively being discussed/investigated in another thread but yes,
we will be marking clang versions affected by this problem as broken for
__counted_by:

  https://lore.kernel.org/20261007-b4-landlock-fix-domain-fortify-panic-v1-1-042e9108f5f8@oss.qualcomm.com/

> >  security/landlock/domain.h | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> > index 60e8eb2d6ac2..4bf1555a4349 100644
> > --- a/security/landlock/domain.h
> > +++ b/security/landlock/domain.h
> > @@ -246,8 +246,12 @@ struct landlock_domain {
> >  			 * properly handle merged rulesets without overlapping
> >  			 * access rights.  These layers are set once and never
> >  			 * changed for the lifetime of the domain.
> > +			 *
> > +			 * Not annotated with __counted_by(@num_layers): clang
> > +			 * miscomputes the counter address for a FAM nested in an
> > +			 * anonymous struct within a union, which trips FORTIFY.
> >  			 */
> > -			struct layer_config layers[] __counted_by(num_layers);
> > +			struct layer_config layers[];
> >  		};
> >  	};
> >  };
> > -- 
> > 2.43.0
> > 
> > 
> 
> -- 
> <https://www.alejandro-colomar.es>



-- 
Cheers,
Nathan

Re: [PATCH] landlock: Drop __counted_by from landlock_domain.layers

[Nguyen Ngoc Thang]

Hi Nathan, Alex,

Thanks for the pointers. I wasn't aware of the earlier thread, so please
disregard my patch in favor of the one from Qualcomm and the Clang
version gating of __counted_by.

To answer Alex's question: I only tested with Clang 22.1.8 (Debian
22.1.8-1~deb13u4, same as the syzbot build). I did not try Clang 23 or
later, so I can't say whether it is fixed there.

Thanks,
Nguyen

Re: [PATCH] landlock: Drop __counted_by from landlock_domain.layers

[Günther Noack]

On Sat, Oct 10, 2026 at 11:44:49PM +0700, Nguyen Ngoc Thang wrote:
> With clang 22 and CONFIG_FORTIFY_SOURCE, the second
> landlock_restrict_self() call hits a bogus fortify report in
> inherit_ruleset():
> 
>   memcpy: detected buffer overflow: 16 byte read of buffer size 0
>   Call Trace:
>    __fortify_panic
>    landlock_merge_ruleset+0xc46/0xc90
>    __se_sys_landlock_restrict_self+0x2bc/0xd40
> 
> layers[] lives in an anonymous struct inside the anonymous union
> that overlays work_free.  For such a flexible array, clang computes the
> address of the __counted_by counter as the start of the array instead of
> &num_layers.  The bound is then read from layers[0], i.e. from
> the first layer's configuration, which is not a count.  The same layout
> crashes clang itself in a standalone test.
> 
> The annotation was added as documentation, so drop it rather than
> restructure the union.  Keep the information in the member comment.
> 
> Reported-by: syzbot+a0eecb22de4d821a9e5d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a0eecb22de4d821a9e5d
> Fixes: bd3a19800dd1 ("landlock: Add counted_by in landlock_domain")
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
> Reproduced with clang 22.1.8 and FORTIFY_SOURCE on mic/next (64b9e6eff524):
> the second landlock_restrict_self() hits the report, while 16 nested
> restricts run clean with this patch.  Mickaël, if you prefer to just drop
> bd3a19800dd1 from next, that works as well.
>  security/landlock/domain.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> index 60e8eb2d6ac2..4bf1555a4349 100644
> --- a/security/landlock/domain.h
> +++ b/security/landlock/domain.h
> @@ -246,8 +246,12 @@ struct landlock_domain {
>  			 * properly handle merged rulesets without overlapping
>  			 * access rights.  These layers are set once and never
>  			 * changed for the lifetime of the domain.
> +			 *
> +			 * Not annotated with __counted_by(@num_layers): clang
> +			 * miscomputes the counter address for a FAM nested in an
> +			 * anonymous struct within a union, which trips FORTIFY.
>  			 */
> -			struct layer_config layers[] __counted_by(num_layers);
> +			struct layer_config layers[];
>  		};
>  	};
>  };
> -- 
> 2.43.0
> 

Reviewed-by: Günther Noack <gnoack3000@gmail.com>

I agree - it seems better to drop the __counted_by rather than
restructuring the entire struct for that.

Thanks also for adding a comment about it.  The __counted_by() has
only been added very recently in September, in commit bd3a19800dd1
("landlock: Add counted_by in landlock_domain").  Better to have a
note about it so that we don't accidentally add it back. :)

–Günther

Re: [PATCH] landlock: Drop __counted_by from landlock_domain.layers

[Mickaël Salaün]

On Sun, Oct 11, 2026 at 03:11:12PM +0200, Günther Noack wrote:
> On Sat, Oct 10, 2026 at 11:44:49PM +0700, Nguyen Ngoc Thang wrote:
> > With clang 22 and CONFIG_FORTIFY_SOURCE, the second
> > landlock_restrict_self() call hits a bogus fortify report in
> > inherit_ruleset():
> > 
> >   memcpy: detected buffer overflow: 16 byte read of buffer size 0
> >   Call Trace:
> >    __fortify_panic
> >    landlock_merge_ruleset+0xc46/0xc90
> >    __se_sys_landlock_restrict_self+0x2bc/0xd40
> > 
> > layers[] lives in an anonymous struct inside the anonymous union
> > that overlays work_free.  For such a flexible array, clang computes the
> > address of the __counted_by counter as the start of the array instead of
> > &num_layers.  The bound is then read from layers[0], i.e. from
> > the first layer's configuration, which is not a count.  The same layout
> > crashes clang itself in a standalone test.
> > 
> > The annotation was added as documentation, so drop it rather than
> > restructure the union.  Keep the information in the member comment.
> > 
> > Reported-by: syzbot+a0eecb22de4d821a9e5d@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=a0eecb22de4d821a9e5d
> > Fixes: bd3a19800dd1 ("landlock: Add counted_by in landlock_domain")
> > Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>

Thanks for the patch.

> > ---
> > Reproduced with clang 22.1.8 and FORTIFY_SOURCE on mic/next (64b9e6eff524):
> > the second landlock_restrict_self() hits the report, while 16 nested
> > restricts run clean with this patch.  Mickaël, if you prefer to just drop
> > bd3a19800dd1 from next, that works as well.
> >  security/landlock/domain.h | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> > index 60e8eb2d6ac2..4bf1555a4349 100644
> > --- a/security/landlock/domain.h
> > +++ b/security/landlock/domain.h
> > @@ -246,8 +246,12 @@ struct landlock_domain {
> >  			 * properly handle merged rulesets without overlapping
> >  			 * access rights.  These layers are set once and never
> >  			 * changed for the lifetime of the domain.
> > +			 *
> > +			 * Not annotated with __counted_by(@num_layers): clang
> > +			 * miscomputes the counter address for a FAM nested in an
> > +			 * anonymous struct within a union, which trips FORTIFY.
> >  			 */
> > -			struct layer_config layers[] __counted_by(num_layers);
> > +			struct layer_config layers[];
> >  		};
> >  	};
> >  };
> > -- 
> > 2.43.0
> > 
> 
> Reviewed-by: Günther Noack <gnoack3000@gmail.com>
> 
> I agree - it seems better to drop the __counted_by rather than
> restructuring the entire struct for that.
> 
> Thanks also for adding a comment about it.  The __counted_by() has
> only been added very recently in September, in commit bd3a19800dd1
> ("landlock: Add counted_by in landlock_domain").  Better to have a
> note about it so that we don't accidentally add it back. :)

Actually, I independently wrote a quite similar patch (while traveling),
but it applies to v7.3-rc5 instead of mic/next.  I removed the patch
series that I applied on mic/next because it conflicts with my patch and
I need to send a new version anyway.  If you don't mind, I'll keep
Günther's Reviewed-by tag and credit other contributors in my patch.
I'll propose it for v7.3-rc7 (today) because it is trivial and will help
new patches on top of that.

Re: [PATCH] landlock: Drop __counted_by from landlock_domain.layers

[Nguyen Ngoc Thang]

Hi Mickaël, Günther,

Sounds good, no problem at all. Thanks for the quick review, and
happy for you to take your version for v7.3-rc7.

Thanks,
Thang

Re: [PATCH] landlock: Drop __counted_by from landlock_domain.layers

[Mickaël Salaün]

On Sun, Oct 11, 2026 at 04:32:05PM +0200, Mickaël Salaün wrote:
> On Sun, Oct 11, 2026 at 03:11:12PM +0200, Günther Noack wrote:
> > On Sat, Oct 10, 2026 at 11:44:49PM +0700, Nguyen Ngoc Thang wrote:
> > > With clang 22 and CONFIG_FORTIFY_SOURCE, the second
> > > landlock_restrict_self() call hits a bogus fortify report in
> > > inherit_ruleset():
> > > 
> > >   memcpy: detected buffer overflow: 16 byte read of buffer size 0
> > >   Call Trace:
> > >    __fortify_panic
> > >    landlock_merge_ruleset+0xc46/0xc90
> > >    __se_sys_landlock_restrict_self+0x2bc/0xd40
> > > 
> > > layers[] lives in an anonymous struct inside the anonymous union
> > > that overlays work_free.  For such a flexible array, clang computes the
> > > address of the __counted_by counter as the start of the array instead of
> > > &num_layers.  The bound is then read from layers[0], i.e. from
> > > the first layer's configuration, which is not a count.  The same layout
> > > crashes clang itself in a standalone test.
> > > 
> > > The annotation was added as documentation, so drop it rather than
> > > restructure the union.  Keep the information in the member comment.
> > > 
> > > Reported-by: syzbot+a0eecb22de4d821a9e5d@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=a0eecb22de4d821a9e5d
> > > Fixes: bd3a19800dd1 ("landlock: Add counted_by in landlock_domain")
> > > Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> 
> Thanks for the patch.
> 
> > > ---
> > > Reproduced with clang 22.1.8 and FORTIFY_SOURCE on mic/next (64b9e6eff524):
> > > the second landlock_restrict_self() hits the report, while 16 nested
> > > restricts run clean with this patch.  Mickaël, if you prefer to just drop
> > > bd3a19800dd1 from next, that works as well.
> > >  security/landlock/domain.h | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> > > index 60e8eb2d6ac2..4bf1555a4349 100644
> > > --- a/security/landlock/domain.h
> > > +++ b/security/landlock/domain.h
> > > @@ -246,8 +246,12 @@ struct landlock_domain {
> > >  			 * properly handle merged rulesets without overlapping
> > >  			 * access rights.  These layers are set once and never
> > >  			 * changed for the lifetime of the domain.
> > > +			 *
> > > +			 * Not annotated with __counted_by(@num_layers): clang
> > > +			 * miscomputes the counter address for a FAM nested in an
> > > +			 * anonymous struct within a union, which trips FORTIFY.
> > >  			 */
> > > -			struct layer_config layers[] __counted_by(num_layers);
> > > +			struct layer_config layers[];
> > >  		};
> > >  	};
> > >  };
> > > -- 
> > > 2.43.0
> > > 
> > 
> > Reviewed-by: Günther Noack <gnoack3000@gmail.com>
> > 
> > I agree - it seems better to drop the __counted_by rather than
> > restructuring the entire struct for that.
> > 
> > Thanks also for adding a comment about it.  The __counted_by() has
> > only been added very recently in September, in commit bd3a19800dd1
> > ("landlock: Add counted_by in landlock_domain").  Better to have a
> > note about it so that we don't accidentally add it back. :)
> 
> Actually, I independently wrote a quite similar patch (while traveling),
> but it applies to v7.3-rc5 instead of mic/next.  I removed the patch
> series that I applied on mic/next because it conflicts with my patch and
> I need to send a new version anyway.  If you don't mind, I'll keep
> Günther's Reviewed-by tag and credit other contributors in my patch.
> I'll propose it for v7.3-rc7 (today) because it is trivial and will help
> new patches on top of that.

Here is my rebase:
https://git.kernel.org/mic/c/3d8cc52db2f86b624308321c58513019f6b21eec