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

["Mickaël Salaün" <mic@digikod.net>]

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.