[PATCH net] net: bridge: don't under-estimate the VLAN tunnel info size

[PATCH net] net: bridge: don't under-estimate the VLAN tunnel info size

[Yiming Qian]

br_info_notify() sizes the notification skb with br_nlmsg_size() and then
fills it with br_fill_ifinfo().  If the fill does not fit, br_info_notify()
does WARN_ON(err == -EMSGSIZE) because that is supposed to mean a bug in
br_nlmsg_size().

br_nlmsg_size() (via br_get_link_af_size_filtered() and
br_get_vlan_tunnel_info_size()) only accounts for the VLANs that have a
tunnel mapping at that moment, while br_fill_vlan_tunnel_info() emits an
attribute for every VLAN that has one by the time it runs.  br_info_notify()
also runs without RTNL (e.g. from br_forward_delay_timer_expired()), so
tunnel mappings added or removed under RTNL between the size calculation and
the fill make the fill need more room than was reserved:

  WARNING: net/bridge/br_netlink.c:660 at br_info_notify+0x13f/0x150
  ...
  Call Trace:
   <IRQ>
   br_forward_delay_timer_expired+0x1b3/0x1f0
   call_timer_fn+0x2d/0xd0
   __run_timer_base+0x5ba/0x7d0
   run_timer_softirq+0x31/0x60
   handle_softirqs+0x17f/0x570
  ...
  Kernel panic - not syncing: kernel: panic_on_warn set ...

Size the tunnel info for the maximum number of attributes that
br_fill_vlan_tunnel_info() can emit for the VLAN group instead of for the
mappings that are currently installed.  Consecutive VIDs with consecutive
tunnel IDs are compressed into two attributes, so the fill never emits more
attributes than there are usable VLANs, which keeps br_nlmsg_size() an upper
bound of what the fill needs.

Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
Cc: Yiming Qian <yimingqian591@gmail.com>
Signed-off-by: Yiming Qian <yimingqian591@gmail.com>
---
 net/bridge/br_netlink_tunnel.c | 44 ++++++++++++----------------------
 1 file changed, 15 insertions(+), 29 deletions(-)

diff --git a/net/bridge/br_netlink_tunnel.c b/net/bridge/br_netlink_tunnel.c
index e7eceab5b515d..74627323711c1 100644
--- a/net/bridge/br_netlink_tunnel.c
+++ b/net/bridge/br_netlink_tunnel.c
@@ -35,39 +35,25 @@ bool vlan_tunid_inrange(const struct net_bridge_vlan *v_curr,
 	return (be32_to_cpu(tunid_curr) - be32_to_cpu(tunid_last)) == 1;
 }
 
-static int __get_num_vlan_tunnel_infos(struct net_bridge_vlan_group *vg)
+/* Upper bound of the number of IFLA_BRIDGE_VLAN_TUNNEL_INFO attributes that
+ * br_fill_vlan_tunnel_info() can emit for @vg.
+ *
+ * br_info_notify() is also called without RTNL (e.g. from the STP timers), so
+ * the set of VLANs that have a tunnel mapping can change between the size
+ * calculation done by br_nlmsg_size() and the actual fill.  Account for the
+ * maximum instead of the currently used mappings: consecutive VIDs with
+ * consecutive tunnel IDs are compressed into two attributes, so the fill never
+ * emits more attributes than there are usable VLANs in the group.
+ */
+static int __get_max_num_vlan_tunnel_infos(struct net_bridge_vlan_group *vg)
 {
-	struct net_bridge_vlan *v, *vtbegin = NULL, *vtend = NULL;
+	struct net_bridge_vlan *v;
 	int num_tinfos = 0;
 
-	/* Count number of vlan infos */
 	list_for_each_entry_rcu(v, &vg->vlan_list, vlist) {
 		/* only a context, bridge vlan not activated */
-		if (!br_vlan_should_use(v) || !v->tinfo.tunnel_id)
-			continue;
-
-		if (!vtbegin) {
-			goto initvars;
-		} else if ((v->vid - vtend->vid) == 1 &&
-			   vlan_tunid_inrange(v, vtend)) {
-			vtend = v;
-			continue;
-		} else {
-			if ((vtend->vid - vtbegin->vid) > 0)
-				num_tinfos += 2;
-			else
-				num_tinfos += 1;
-		}
-initvars:
-		vtbegin = v;
-		vtend = v;
-	}
-
-	if (vtbegin && vtend) {
-		if ((vtend->vid - vtbegin->vid) > 0)
-			num_tinfos += 2;
-		else
-			num_tinfos += 1;
+		if (br_vlan_should_use(v))
+			num_tinfos++;
 	}
 
 	return num_tinfos;
@@ -81,7 +67,7 @@ int br_get_vlan_tunnel_info_size(struct net_bridge_vlan_group *vg)
 		return 0;
 
 	rcu_read_lock();
-	num_tinfos = __get_num_vlan_tunnel_infos(vg);
+	num_tinfos = __get_max_num_vlan_tunnel_infos(vg);
 	rcu_read_unlock();
 
 	return num_tinfos * __get_vlan_tinfo_size();
-- 
2.34.1

Re: [PATCH net] net: bridge: don't under-estimate the VLAN tunnel info size

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net: bridge: don't under-estimate the VLAN tunnel info size

[Ido Schimmel]

On Wed, Oct 07, 2026 at 05:59:37AM +0000, Yiming Qian wrote:
> br_info_notify() sizes the notification skb with br_nlmsg_size() and then
> fills it with br_fill_ifinfo().  If the fill does not fit, br_info_notify()
> does WARN_ON(err == -EMSGSIZE) because that is supposed to mean a bug in
> br_nlmsg_size().
> 
> br_nlmsg_size() (via br_get_link_af_size_filtered() and
> br_get_vlan_tunnel_info_size()) only accounts for the VLANs that have a
> tunnel mapping at that moment, while br_fill_vlan_tunnel_info() emits an
> attribute for every VLAN that has one by the time it runs.  br_info_notify()
> also runs without RTNL (e.g. from br_forward_delay_timer_expired()), so
> tunnel mappings added or removed under RTNL between the size calculation and
> the fill make the fill need more room than was reserved:
> 
>   WARNING: net/bridge/br_netlink.c:660 at br_info_notify+0x13f/0x150
>   ...
>   Call Trace:
>    <IRQ>
>    br_forward_delay_timer_expired+0x1b3/0x1f0
>    call_timer_fn+0x2d/0xd0
>    __run_timer_base+0x5ba/0x7d0
>    run_timer_softirq+0x31/0x60
>    handle_softirqs+0x17f/0x570
>   ...
>   Kernel panic - not syncing: kernel: panic_on_warn set ...
> 
> Size the tunnel info for the maximum number of attributes that
> br_fill_vlan_tunnel_info() can emit for the VLAN group instead of for the
> mappings that are currently installed.  Consecutive VIDs with consecutive
> tunnel IDs are compressed into two attributes, so the fill never emits more
> attributes than there are usable VLANs, which keeps br_nlmsg_size() an upper
> bound of what the fill needs.
> 
> Fixes: efa5356b0d97 ("bridge: per vlan dst_metadata netlink support")
> Cc: Yiming Qian <yimingqian591@gmail.com>
> Signed-off-by: Yiming Qian <yimingqian591@gmail.com>

br_info_notify() is inherently racy when called without RTNL. Better to
just remove the WARN_ON() in br_info_notify(). Blaming commit
b7853d73e39b ("bridge: add vlan info to bridge setlink and dellink
notification messages") seems appropriate.

Note: "don’t repost your patches within one 24h period"
https://docs.kernel.org/next/process/maintainer-netdev.html

Re: [PATCH net] net: bridge: don't under-estimate the VLAN tunnel info size

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete.
- [High] The commit message says the change 'keeps br_nlmsg_size() an
  upper bound of what the fill needs', but the IFLA_BRIDGE_VLAN_INFO…
- [Medium] On any bridge port with vlan_tunnel enabled,
  br_get_vlan_tunnel_info_size() now reserves __get_vlan_tinfo_size()…

--

net: bridge: don't under-estimate the VLAN tunnel info size

br_get_vlan_tunnel_info_size() used to count the compressed ranges of VLANs
that currently have a tunnel mapping. This patch makes it reserve one
IFLA_BRIDGE_VLAN_TUNNEL_INFO attribute for every usable VLAN in the group.
The goal is to stop br_info_notify() from hitting
WARN_ON(err == -EMSGSIZE) when mappings change between br_nlmsg_size() and
br_fill_ifinfo() on the STP timer path, which runs without RTNL.

> Size the tunnel info for the maximum number of attributes that
> br_fill_vlan_tunnel_info() can emit for the VLAN group instead of for the
> mappings that are currently installed.  Consecutive VIDs with consecutive
> tunnel IDs are compressed into two attributes, so the fill never emits more
> attributes than there are usable VLANs, which keeps br_nlmsg_size() an upper
> bound of what the fill needs.

[Severity: High]
Does br_nlmsg_size() really become an upper bound with this change? The
IFLA_BRIDGE_VLAN_INFO part of the same message is still sized from a
snapshot.

br_get_link_af_size_filtered() gets that size from __get_num_vlan_infos().
That function counts compressed ranges using the current PVID, the
BRIDGE_VLAN_INFO_UNTAGGED flag of each VLAN and the current VLAN set:

net/bridge/br_netlink.c:br_get_link_af_size_filtered() {
    ...
	num_vlan_infos = br_get_num_vlan_infos(vg, filter_mask);
    ...
	vinfo_sz += num_vlan_infos * nla_total_size(sizeof(struct bridge_vlan_info));
    ...
}

br_fill_ifvlaninfo_compressed() later works out the ranges again from live
state. br_ifinfo_notify() always passes RTEXT_FILTER_BRVLAN_COMPRESSED,
including on the br_forward_delay_timer_expired() path in the trace.

Suppose an RTM_SETLINK under RTNL does one of the following while the timer
is between sizing and filling:

- toggles UNTAGGED on a VLAN in the middle of a range (one 2-attribute
  range becomes 2+1+2)
- changes the PVID
- adds non-adjacent VLANs through __vlan_add()->__vlan_add_list()

Can't br_fill_ifvlaninfo_compressed() still return -EMSGSIZE and hit the
same WARN_ON in br_info_notify()? This applies to tunnel and non-tunnel
ports alike.

About 30 extra 8-byte attributes are enough to go past the kmalloc slack.
A single setlink with many IFLA_BRIDGE_VLAN_INFO entries can produce that
many.

The patch only makes the tunnel half of br_get_link_af_size_filtered()
pessimistic. The two halves now follow different sizing rules.

> diff --git a/net/bridge/br_netlink_tunnel.c b/net/bridge/br_netlink_tunnel.c
> index e7eceab5b515d..74627323711c1 100644
> --- a/net/bridge/br_netlink_tunnel.c
> +++ b/net/bridge/br_netlink_tunnel.c
> @@ -35,39 +35,25 @@ bool vlan_tunid_inrange(const struct net_bridge_vlan *v_curr,
>  	return (be32_to_cpu(tunid_curr) - be32_to_cpu(tunid_last)) == 1;
>  }
>  
> -static int __get_num_vlan_tunnel_infos(struct net_bridge_vlan_group *vg)
> +/* Upper bound of the number of IFLA_BRIDGE_VLAN_TUNNEL_INFO attributes that
> + * br_fill_vlan_tunnel_info() can emit for @vg.
> + *
> + * br_info_notify() is also called without RTNL (e.g. from the STP timers), so
> + * the set of VLANs that have a tunnel mapping can change between the size
> + * calculation done by br_nlmsg_size() and the actual fill.  Account for the
> + * maximum instead of the currently used mappings: consecutive VIDs with
> + * consecutive tunnel IDs are compressed into two attributes, so the fill never
> + * emits more attributes than there are usable VLANs in the group.
> + */
> +static int __get_max_num_vlan_tunnel_infos(struct net_bridge_vlan_group *vg)
>  {
> -	struct net_bridge_vlan *v, *vtbegin = NULL, *vtend = NULL;
> +	struct net_bridge_vlan *v;
>  	int num_tinfos = 0;
>  
> -	/* Count number of vlan infos */
>  	list_for_each_entry_rcu(v, &vg->vlan_list, vlist) {
>  		/* only a context, bridge vlan not activated */
> -		if (!br_vlan_should_use(v) || !v->tinfo.tunnel_id)
> -			continue;

[ ... ]

> +		if (br_vlan_should_use(v))
> +			num_tinfos++;
>  	}

[Severity: High]
Isn't this still only a bound on vlan_list as it looks at sizing time?

When called from br_forward_delay_timer_expired(), br_info_notify() holds
only br->lock and RCU. br_fill_ifinfo()->br_fill_vlan_tunnel_info() then
walks the list again and checks BR_VLAN_TUNNEL_BIT again on its own:

net/bridge/br_netlink.c:br_info_notify() {
    ...
	skb = nlmsg_new(br_nlmsg_size(dev, filter), GFP_ATOMIC);
    ...
	err = br_fill_ifinfo(skb, port, 0, 0, event, 0, filter, dev, false);
	if (err < 0) {
		/* -EMSGSIZE implies BUG in br_nlmsg_size() */
		WARN_ON(err == -EMSGSIZE);
    ...
}

The RTNL writers don't take br->lock, so two inputs can still change
between those steps.

First, VLANs can be added. __vlan_add()->__vlan_add_list() publishes them
with list_add_rcu() under RTNL only. One RTM_SETLINK can carry
IFLA_BRIDGE_VLAN_INFO followed by IFLA_BRIDGE_VLAN_TUNNEL_INFO for the new
VIDs, and br_afspec() handles both. A VLAN that wasn't on the list when
this loop ran isn't counted, but br_fill_vlan_tunnel_info() will emit it.

Second, the flag can change. br_setport() sets BR_VLAN_TUNNEL with a
lockless try_cmpxchg(), and br_setlink() then calls br_afspec() to install
mappings. If sizing saw the bit clear, nothing is reserved:

net/bridge/br_netlink.c:br_get_link_af_size_filtered() {
    ...
	if (p && test_bit(BR_VLAN_TUNNEL_BIT, &p->flags))
		vinfo_sz += br_get_vlan_tunnel_info_size(vg);
    ...
}

The fill can then see the bit set and emit one 28-byte attribute per
mapped VLAN:

net/bridge/br_netlink.c:br_fill_ifinfo() {
    ...
		if (port && test_bit(BR_VLAN_TUNNEL_BIT, &port->flags))
			err = br_fill_vlan_tunnel_info(skb, vg);
    ...
}

In either case, wouldn't br_fill_vlan_tinfo() return -EMSGSIZE and hit the
WARN_ON quoted in the commit message? Nine or more tunnel attributes that
can't be merged would exceed the usual kmalloc slack. This needs
CAP_NET_ADMIN in a network namespace, which includes unprivileged user
namespaces.

The "upper bound" in the new comment only holds while the VLAN list and
the flag stay the same, and nothing guarantees that without RTNL.

Could the size and the fill be computed from one consistent view, or use a
bound that doesn't depend on current membership? Another option is for the
non-RTNL callers to stop treating -EMSGSIZE as a bug, for example by
retrying with a larger skb.

[ ... ]

> @@ -81,7 +67,7 @@ int br_get_vlan_tunnel_info_size(struct net_bridge_vlan_group *vg)
>  		return 0;
>  
>  	rcu_read_lock();
> -	num_tinfos = __get_num_vlan_tunnel_infos(vg);
> +	num_tinfos = __get_max_num_vlan_tunnel_infos(vg);
>  	rcu_read_unlock();
>  
>  	return num_tinfos * __get_vlan_tinfo_size();

[Severity: Medium]
On a vlan_tunnel port, this now reserves __get_vlan_tinfo_size() (28
bytes) for every usable VLAN, whether it is mapped or not. Two examples:

- VLANs 1000-1999 mapped to consecutive VNIs go from 56 bytes to about
  28KB.
- A port with 4094 VLANs and few mappings now reserves about 114KB.

Is this cost intended on the br_info_notify() path itself? That path calls
nlmsg_new(br_nlmsg_size(), GFP_ATOMIC) from STP timer softirq context, so
these sizes become order-3 to order-5 atomic page allocations. When one
fails, the notification is dropped and rtnl_set_sk_err() reports ENOBUFS.

The inflated size also reaches messages outside the bridge code.
br_get_link_af_size_filtered() adds it regardless of filter_mask. It is
registered as get_link_af_size in br_af_ops, which has no fill_link_af:

static struct rtnl_af_ops br_af_ops __read_mostly = {
	.family			= AF_BRIDGE,
	.get_link_af_size	= br_get_link_af_size_filtered,
};

That has two effects:

- if_nlmsg_size()->rtnl_link_get_af_size() reserves the space for every
  generic rtmsg_ifinfo() or RTM_GETLINK message for the port, and nothing
  ever fills it.
- rtnl_calcit() takes the max of if_nlmsg_size() over every netdev in the
  netns as min_dump_alloc for RTM_GETLINK dumps that carry IFLA_EXT_MASK.
  One such port inflates those dumps for the whole namespace.

The commit message doesn't mention this cost. As noted above, the
over-reservation still doesn't guarantee the bound.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007055939.63100-1-yimingqian591%40gmail.com