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

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

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