[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.