Re: [PATCH net-next 06/11] tools: ynl-gen: don't validate nested array attribute types

[Jakub Kicinski <kuba@kernel.org>]

On Sat, 6 Sep 2025 13:22:01 +0000 Asbjørn Sloth Tønnesen wrote:
> > I don't understand, please provide more details.
> > This is an ArrayNest, right?
> > 
> > [ARRAY-ATTR]
> >    [ENTRY]
> >      [MEMBER1]
> >      [MEMBER2]
> >    [ENTRY]
> >      [MEMBER1]
> >      [MEMBER2]
> > 
> > Which level are you saying doesn't matter?
> > If entry is a nest it must be a valid nest.
> > What the comment you're quoting is saying is that the nla_type of ENTRY
> > doesn't matter.  
> 
> I will expand this in v2, but the gist of it is that this is part of the
> "split attribute counting, and later allocating an array to hold them" code.
> 
> The check that I remove for nested arrays, is an early exit during the
> counting phase. Later in the allocation and parse phase it validates the
> nested payload.
> 
> In include/uapi/linux/wireguard.h:
>  > WGDEVICE_A_PEERS: NLA_NESTED
>  >   0: NLA_NESTED
>  >     WGPEER_A_PUBLIC_KEY: NLA_EXACT_LEN, len WG_KEY_LEN
>  >     [..]
>  >   0: NLA_NESTED
>  >     ...
>  >   ...  
> 
> The current check requires that the nested type is valid in the nested
> attribute set, which in this case resolves to WGDEVICE_A_UNSPEC, which is
> YNL_PT_REJECT, and it takes the early exit and returns YNL_PARSE_CB_ERROR.

I see your point now. We're validating ENTRY as an attribute in the
parent attribute set, but it's just a meaningless id.

I think we need more fixing here. The real parsing loop will only
validate what's _inside_ the [MEMBER]. Which doesn't matter all
that much to nests, but look at what happens if subtype is a scalar.
We'll just call ynl_attr_get_u32(), type is never really validate.

I think we need this, and make the codegen feed in the ARRAY-ATTR type
to validate ENTRY?

diff --git a/tools/net/ynl/lib/ynl.c b/tools/net/ynl/lib/ynl.c
index 2a169c3c0797..e43167398c69 100644
--- a/tools/net/ynl/lib/ynl.c
+++ b/tools/net/ynl/lib/ynl.c
@@ -360,15 +360,15 @@ static int ynl_cb_done(const struct nlmsghdr *nlh, struct ynl_parse_arg *yarg)
 
 /* Attribute validation */
 
-int ynl_attr_validate(struct ynl_parse_arg *yarg, const struct nlattr *attr)
+int __ynl_attr_validate(struct ynl_parse_arg *yarg, const struct nlattr *attr,
+			unsigned int type)
 {
 	const struct ynl_policy_attr *policy;
-	unsigned int type, len;
 	unsigned char *data;
+	unsigned int len;
 
 	data = ynl_attr_data(attr);
 	len = ynl_attr_data_len(attr);
-	type = ynl_attr_type(attr);
 	if (type > yarg->rsp_policy->max_attr) {
 		yerr(yarg->ys, YNL_ERROR_INTERNAL,
 		     "Internal error, validating unknown attribute");
@@ -450,6 +450,11 @@ int ynl_attr_validate(struct ynl_parse_arg *yarg, const struct nlattr *attr)
 	return 0;
 }
 
+int ynl_attr_validate(struct ynl_parse_arg *yarg, const struct nlattr *attr)
+{
+	return __ynl_attr_validate(yarg, attr, ynl_attr_type(attr));
+}
+
 int ynl_submsg_failed(struct ynl_parse_arg *yarg, const char *field_name,
 		      const char *sel_name)
 {