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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d612ce20-ae4a-4f6d-9d1b-a3d56f3d10a9@fiberby.net>
Date: Sat, 6 Sep 2025 15:10:37 +0000
From: Asbjørn Sloth Tønnesen <ast@...erby.net>
To: Jacob Keller <jacob.e.keller@...el.com>,
 "Jason A. Donenfeld" <Jason@...c4.com>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Cc: Donald Hunter <donald.hunter@...il.com>, Simon Horman <horms@...nel.org>,
 Andrew Lunn <andrew+netdev@...n.ch>, wireguard@...ts.zx2c4.com,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 Johannes Berg <johannes.berg@...el.com>
Subject: Re: [PATCH net-next 06/11] tools: ynl-gen: don't validate nested
 array attribute types

CC: Johannes

On 9/6/25 12:24 AM, Jacob Keller wrote:
> On 9/4/2025 3:01 PM, Asbjørn Sloth Tønnesen wrote:
>> In nested arrays don't require that the intermediate
>> attribute type should be a valid attribute type, it
>> might just be an index or simple 0, it is often not
>> even used.
>>
>> See include/net/netlink.h about NLA_NESTED_ARRAY:
>>> The difference to NLA_NESTED is the structure:
>>> NLA_NESTED has the nested attributes directly inside
>>> while an array has the nested attributes at another
>>> level down and the attribute types directly in the
>>> nesting don't matter.
>>
> 
> To me, it would seem like it makes more sense to define these (even if
> thats defined per family?) than to just say they aren't defined at all?
> 
> Hm.

I considered adding some of that metadata too, as I am actually removing
it for wireguard (in comment form, but still).

In include/uapi/linux/wireguard.h in the comment block at the top, it is
very clear that wireguard only used type 0 for all the nested array
entries, however the truth is that it doesn't care. It therefore doesn't
matter if the generated -user.* keeps track of the index in .idx, or that
cli.py decodes a JSON array and sends it with indexes, it's not needed,
but it still works.

In practice I don't think we will break any clients if we enforced it, and
validated that wireguard only accepts type 0 entries, in it's nested arrays.

For the other families, I don't know how well defined it is, Johannes have
stated that nl80211 doesn't care which types are used, but I have no idea
how consistent clients have abused that statement to send random data,
or do they all just send zeros?

This would make a lot more sense if 'array-nest' hadn't been renamed to
'indexed-array' in ynl, because it feels wrong to add 'unindexed: true' now.
We could also call it 'all-zero-indexed: true'.

In cli.py this gives some extra issues, as seen in [1], the nested arrays
are outputted as '[{0: {..}}, {0: {..}}, ..]', but on input has the format
'[{..},{..}, ..]' because it has to be JSON-compatible on input.

If we had an attribute like 'all-zero-indexed' then cli.py, could also output
'[{..},{..}, ..]'.

[1] https://lore.kernel.org/netdev/20250904220255.1006675-3-ast@fiberby.net/