Re: [PATCH net] net: bridge: prevent too big nested attributes in br_fill_linkxstats()

[Ido Schimmel <idosch@nvidia.com>]

On Mon, May 18, 2026 at 01:05:31PM +0000, Eric Dumazet wrote:
> After commit ff205bf8c554 ("netlink: add one debug check in nla_nest_end()")
> syzbot found that br_fill_linkxstats() can send corrupted netlink packets.
> 
> Make sure the nested attribute size is bounded.
> 
> Fixes: a60c090361ea ("bridge: netlink: export per-vlan stats")
> Reported-by: syzbot+a35f9259d08f907c06e6@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/6a0b0da3.050a0220.175f0c.0000.GAE@google.com/
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> Cc: Nikolay Aleksandrov <razor@blackwall.org>
> Cc: Ido Schimmel <idosch@nvidia.com>
> ---
>  net/bridge/br_netlink.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 6fd5386a1d646542c184702e13cc2e6c8ee1820d..e15a08a34aeab2429b6c49c5a0ecab9b47582f06 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -1827,6 +1827,7 @@ static int br_fill_linkxstats(struct sk_buff *skb,
>  	struct nlattr *nla __maybe_unused;
>  	struct net_bridge_port *p = NULL;
>  	struct net_bridge_vlan_group *vg;
> +	unsigned int limit = U16_MAX;
>  	struct net_bridge_vlan *v;
>  	struct net_bridge *br;
>  	struct nlattr *nest;
> @@ -1841,6 +1842,7 @@ static int br_fill_linkxstats(struct sk_buff *skb,
>  		p = br_port_get_rtnl(dev);
>  		if (!p)
>  			return 0;
> +		limit -= nla_total_size_64bit(sizeof(p->stp_xstats));
>  		br = p->br;
>  		vg = nbp_vlan_group(p);
>  		break;
> @@ -1855,6 +1857,9 @@ static int br_fill_linkxstats(struct sk_buff *skb,
>  	if (vg) {
>  		u16 pvid;
>  
> +		limit -= nla_total_size(sizeof(struct br_mcast_stats)) +
> +			 nla_total_size_64bit(sizeof(struct br_mcast_stats));
> +
>  		pvid = br_get_pvid(vg);
>  		list_for_each_entry(v, &vg->vlan_list, vlist) {
>  			struct bridge_vlan_xstats vxi;
> @@ -1862,6 +1867,10 @@ static int br_fill_linkxstats(struct sk_buff *skb,
>  
>  			if (++vl_idx < *prividx)
>  				continue;
> +
> +			if (skb_tail_pointer(skb) - (unsigned char *)nest >= limit)
> +				goto nla_put_failure;
> +
>  			memset(&vxi, 0, sizeof(vxi));
>  			vxi.vid = v->vid;
>  			vxi.flags = v->flags;

Thanks for the patch. A few things:

1. I used [1] to reproduce the issue. I can confirm that without the
patch (but with ff205bf8c554) the warning is triggered.

2. After applying the fix we get a different warning [2] due to
EMSGSIZE. I believe the WARN_ON() in rtnl_stats_get() should be removed.

3. I am aware that the double accounting of the multicast stats makes
the patch correct, but it looks like a mistake. I find something like
[3] clearer (on top of your patch). 

[1]
ip link add name br1 up type bridge vlan_filtering 1 vlan_default_pvid 0
ip link add name dummy1 up master br1 type dummy
for i in {1..4094}; do bridge vlan add vid $i dev dummy1; done
ip stats show dev dummy1

[2]
WARNING: net/core/rtnetlink.c:6332 at rtnl_stats_get+0x294/0x2c0, CPU#4: ip/4404

[3]
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index e15a08a34aea..eb1292d67f4d 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1842,7 +1842,6 @@ static int br_fill_linkxstats(struct sk_buff *skb,
                p = br_port_get_rtnl(dev);
                if (!p)
                        return 0;
-               limit -= nla_total_size_64bit(sizeof(p->stp_xstats));
                br = p->br;
                vg = nbp_vlan_group(p);
                break;
@@ -1850,6 +1849,16 @@ static int br_fill_linkxstats(struct sk_buff *skb,
                return -EINVAL;
        }
 
+       /* Limit the amount of VLAN stats we put in a message so that both the
+        * inner nest (LINK_XSTATS_TYPE_BRIDGE) and the outer nest
+        * (IFLA_STATS_LINK_XSTATS{,_SLAVE}) will not overflow.
+        */
+       limit -= nla_total_size(0) + /* IFLA_STATS_LINK_XSTATS{,_SLAVE} */
+#ifdef CONFIG_BRIDGE_IGMP_SNOOPING
+                nla_total_size_64bit(sizeof(struct br_mcast_stats)) +
+#endif
+                (p ? nla_total_size_64bit(sizeof(p->stp_xstats)) : 0);
+
        nest = nla_nest_start_noflag(skb, LINK_XSTATS_TYPE_BRIDGE);
        if (!nest)
                return -EMSGSIZE;
@@ -1857,9 +1866,6 @@ static int br_fill_linkxstats(struct sk_buff *skb,
        if (vg) {
                u16 pvid;
 
-               limit -= nla_total_size(sizeof(struct br_mcast_stats)) +
-                        nla_total_size_64bit(sizeof(struct br_mcast_stats));
-
                pvid = br_get_pvid(vg);
                list_for_each_entry(v, &vg->vlan_list, vlist) {
                        struct bridge_vlan_xstats vxi;
@@ -1868,7 +1874,8 @@ static int br_fill_linkxstats(struct sk_buff *skb,
                        if (++vl_idx < *prividx)
                                continue;
 
-                       if (skb_tail_pointer(skb) - (unsigned char *)nest >= limit)
+                       if (skb_tail_pointer(skb) - (unsigned char *)nest +
+                           nla_total_size(sizeof(vxi)) >= limit)
                                goto nla_put_failure;
 
                        memset(&vxi, 0, sizeof(vxi));