Re: [PATCH net-next] net: bridge: bound the port VLAN lists in the IFLA_AF_SPEC nest

[Nikolay Aleksandrov <razor@blackwall.org>]

On 07/09/2026 01:42, Artem Lytkin wrote:
> br_fill_ifinfo() opens one IFLA_AF_SPEC nest, fills it with the VLAN
> information of a bridge port and closes it with nla_nest_end(), which
> stores the accumulated length into nla_len. That field is a u16, so for
> a nest larger than 65535 bytes the stored length wraps modulo 65536.
> The encoding has never been able to describe more than that; nothing
> regressed.
> 
> Three per-VLAN lists share the one nest. An IFLA_BRIDGE_VLAN_INFO costs
> 8 bytes, an IFLA_BRIDGE_VLAN_TUNNEL_INFO 28 and an IFLA_BRIDGE_MST_ENTRY
> 20, on every architecture. A port takes at most 4094 VLANs, so the VLAN
> list alone is at most 32752 bytes, but the other two reach 65535: 2341
> tunnel mappings that are not consecutive in both VID and tunnel id, or
> 1821 with uncompressed VLAN information requested alongside, or 3277
> distinct MSTIs. A VXLAN leaf switch with one VXLAN device maps thousands
> of VLANs to VNIs, and "bridge vlan tunnelshow" and "bridge mst show"
> read these lists through the link dump.
> 
> Nothing fails on the way there. br_get_link_af_size_filtered() accounts
> for all three lists and feeds both rtnl_calcit() for RTM_GETLINK dumps
> and nlmsg_new() in br_info_notify(), so the skb is large enough and no
> nla_put() fails. Userspace then walks the message with RTA_NEXT(),
> which advances by the stored length, so parsing resumes inside VLAN
> payload and everything after the nest is read out of it. A
> CONFIG_DEBUG_NET kernel warns once, in nla_nest_end(), via the check
> added in commit ff205bf8c554 ("netlink: add one debug check in
> nla_nest_end()").
> 
> Measured on a port with 4093 VLAN to VNI mappings: the RTM_GETLINK reply
> for the port is 147776 bytes and carries an IFLA_AF_SPEC nla_len of
> 16280, and the top level attribute walk derails 16672 bytes in. The
> notification path is worse, because it broadcasts. A port flag change
> goes through br_ifinfo_notify(), which asks for compressed VLAN
> information, and the resulting RTM_NEWLINK is 115048 bytes with an
> nla_len of 49088; walking it yields 2342 attributes after the nest,
> 2341 of them read as IFLA_IFNAME, since IFLA_BRIDGE_VLAN_TUNNEL_INFO and
> IFLA_BRIDGE_VLAN_TUNNEL_FLAGS carry the same numeric type. Every
> RTNLGRP_LINK listener in the netns receives that. The DEBUG_NET warning
> fired from br_fill_ifinfo() while the mappings were being added, in the
> context of the process adding them, since each addition notifies too.
> 
> Bound the three lists by length: stop adding entries once the next one
> would not fit in 65535 bytes, counted from the start of the nest so that
> the lists and the inner IFLA_BRIDGE_MST nest share the budget, and
> charge a range at its real cost of one or two entries. This is what
> commit bdd39576bf50a ("net: bridge: prevent too big nested attributes
> in br_fill_linkxstats()") did for the sibling nest in this file. A fixed
> cap on the number of entries, as IFLA_VFINFO_LIST got in
> commit 51e15308c6ae ("rtnetlink: cap IFLA_VFINFO_LIST at a documented
> number of VFs"), does not fit here: the lists share one nest, 4094 VLAN
> entries take half of it and 2340 tunnel entries fill it, so any set of
> per-list caps that is safe in the worst case cuts configurations that
> are described correctly today. The byte budget regresses none of them,
> and it is still a limit userspace can compute, since the per-entry
> costs are fixed. Documentation/networking/bridge.rst states them, along
> with the RTM_GETVLAN interface that does not have this limit.
> 
> The MRP and CFM lists that br_fill_ifinfo() emits for the bridge device
> itself are not touched. They live in the same nest but never next to
> the port lists, and CFM peer status can grow past the limit on its own;
> that is a separate change.
> 
> br_get_link_af_size_filtered() clamps the three lists to the same 65535
> bytes, so a dump of such a port no longer sizes the skb at 147 KB for a
> nest that holds 64 KB, and br_info_notify() no longer makes an
> oversized GFP_ATOMIC allocation from STP timer context for a message
> that would be truncated anyway. The clamp cannot cut into the MRP and
> CFM information: for the bridge device the VLAN list is the only one of
> the three and stays under 33 KB.
> 
> With the bound, the same port dumps as 65932 bytes with an nla_len of
> 65508, the notification is 65936 bytes with 65512, both walk to the
> end, and nothing warns. A port above the bound reports shortened lists,
> so "bridge vlan tunnelshow" and "bridge mst show" print the first
> entries and stop instead of printing garbage, and the rest of the
> message parses. Read such a port with RTM_GETVLAN, which gives each
> entry its own attribute and continues in a new message once one fills
> up.
> 
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
> ---
>   Documentation/networking/bridge.rst | 17 ++++++++++
>   net/bridge/br_mst.c                 | 12 +++----
>   net/bridge/br_netlink.c             | 49 ++++++++++++++++++++++++-----
>   net/bridge/br_netlink_tunnel.c      | 20 ++++++++++--
>   net/bridge/br_private.h             | 23 ++++++++++++--
>   net/bridge/br_private_tunnel.h      |  2 +-
>   6 files changed, 104 insertions(+), 19 deletions(-)
> 

So much crap in a single patch, my eyes hurt... Yes, we know about this problem
and it is obvious to anyone who can count. This is not the right way to fix it,
it should probably be considered a corner case but still - IMO silently truncating
is very bad. I prefer a more explicit path - return error for both (RTM_GETLINK and notifications)
you'll have to plumb down extack but then user-space will know there are too many objects or lost
RTNL_LINK notifications and it should resync (e.g. use RTM_GETVLAN to dump the vlans if needed).

Cheers,
  Nik