netdev - Re: [PATCH v6 net-next 05/14] bridge: Add filling forward path from port to port

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: <20250211132832.aiy6ocvqppoqkd65@skbuf>
Date: Tue, 11 Feb 2025 15:28:32 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Eric Woudstra <ericwouds@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	Pablo Neira Ayuso <pablo@...filter.org>,
	Jozsef Kadlecsik <kadlec@...filter.org>,
	Jiri Pirko <jiri@...nulli.us>, Ivan Vecera <ivecera@...hat.com>,
	Roopa Prabhu <roopa@...dia.com>,
	Nikolay Aleksandrov <razor@...ckwall.org>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Kuniyuki Iwashima <kuniyu@...zon.com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Lorenzo Bianconi <lorenzo@...nel.org>,
	Joe Damato <jdamato@...tly.com>,
	Alexander Lobakin <aleksander.lobakin@...el.com>,
	Frank Wunderlich <frank-w@...lic-files.de>,
	Daniel Golle <daniel@...rotopia.org>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, netfilter-devel@...r.kernel.org,
	coreteam@...filter.org, bridge@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH v6 net-next 05/14] bridge: Add filling forward path from
 port to port

On Sun, Feb 09, 2025 at 12:10:25PM +0100, Eric Woudstra wrote:
> @@ -1453,7 +1454,10 @@ void br_vlan_fill_forward_path_pvid(struct net_bridge *br,
>  	if (!br_opt_get(br, BROPT_VLAN_ENABLED))
>  		return;
>  
> -	vg = br_vlan_group(br);
> +	if (p)
> +		vg = nbp_vlan_group(p);
> +	else
> +		vg = br_vlan_group(br);
>  
>  	if (idx >= 0 &&
>  	    ctx->vlan[idx].proto == br->vlan_proto) {

I think the original usage of br_vlan_group() here was incorrect, and so
is the new usage of nbp_vlan_group(). They should be br_vlan_group_rcu()
and nbp_vlan_group_rcu().

The lockdep annotation is important, otherwise I get this with CONFIG_PROVE_LOCKING=y:
[ 1140.931869] =============================
[ 1140.935996] WARNING: suspicious RCU usage
[ 1140.940094] 6.14.0-rc1-00224-gda8926a49ba1-dirty #2666 Not tainted
[ 1140.946371] -----------------------------
[ 1140.950520] net/bridge/br_private.h:1604 suspicious rcu_dereference_protected() usage!
[ 1140.958622]
[ 1140.958622] other info that might help us debug this:
[ 1140.958622]
[ 1140.966752]
[ 1140.966752] rcu_scheduler_active = 2, debug_locks = 1
[ 1140.973435] 2 locks held by swapper/0/0:
[ 1140.977521]  #0: ffffd9f646c333b0 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire+0x4/0x40
[ 1140.986404]  #1: ffffd9f646c333b0 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire+0x4/0x48
[ 1140.995170]
[ 1140.995170] stack backtrace:
[ 1140.999636] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.14.0-rc1-00224-gda8926a49ba1-dirty #2666
[ 1140.999650] Hardware name: LS1028A RDB Board (DT)
[ 1140.999656] Call trace:
[ 1140.999660]  show_stack+0x24/0x38 (C)
[ 1140.999683]  dump_stack_lvl+0x40/0xa0
[ 1140.999698]  dump_stack+0x18/0x24
[ 1140.999711]  lockdep_rcu_suspicious+0x174/0x218
[ 1140.999723]  br_vlan_fill_forward_path_pvid+0x90/0x150
[ 1140.999735]  br_fill_forward_path+0x54/0x1b0
[ 1140.999751]  dev_fill_bridge_path+0x9c/0x188
[ 1140.999766]  nft_dev_fill_bridge_path+0x2ac/0x418
[ 1140.999785]  nft_flow_offload_bridge_init+0x188/0x1c8
[ 1140.999801]  nft_flow_offload_eval+0x18c/0x300
[ 1140.999816]  nft_do_chain+0x1c8/0x538
[ 1140.999831]  nft_do_chain_bridge+0x15c/0x210
[ 1140.999846]  nf_hook_slow+0x80/0x130
[ 1140.999862]  NF_HOOK+0xd8/0x1d0
[ 1140.999871]  __br_forward+0x138/0x1a0
[ 1140.999880]  br_forward+0xd8/0x160
[ 1140.999889]  br_handle_frame_finish+0x3bc/0x5a8
[ 1140.999900]  br_nf_pre_routing_finish+0x538/0x608
[ 1140.999917]  NF_HOOK+0x254/0x298
[ 1140.999933]  br_nf_pre_routing+0x3e8/0x428
[ 1140.999949]  br_handle_frame+0x264/0x490
[ 1140.999959]  __netif_receive_skb_core+0x13c/0x1128
[ 1140.999975]  __netif_receive_skb_list_core+0xd4/0x1e8
[ 1140.999989]  netif_receive_skb_list_internal+0x224/0x338
[ 1141.000000]  napi_complete_done+0xb4/0x1d8
[ 1141.000012]  gro_cell_poll+0x94/0xb8
[ 1141.000025]  __napi_poll+0x58/0x258
[ 1141.000040]  net_rx_action+0x1f4/0x3e0
[ 1141.000055]  handle_softirqs+0x184/0x458
[ 1141.000070]  __do_softirq+0x20/0x2c
[ 1141.000079]  ____do_softirq+0x1c/0x30
[ 1141.000095]  call_on_irq_stack+0x24/0x58
[ 1141.000111]  do_softirq_own_stack+0x28/0x40
[ 1141.000127]  __irq_exit_rcu+0xd4/0x1a0
[ 1141.000139]  irq_exit_rcu+0x1c/0x40
[ 1141.000152]  el1_interrupt+0x8c/0xc0
[ 1141.000170]  el1h_64_irq_handler+0x18/0x28
[ 1141.000186]  el1h_64_irq+0x6c/0x70
[ 1141.000195]  arch_local_irq_enable+0x8/0x10 (P)
[ 1141.000213]  cpuidle_enter+0x44/0x68
[ 1141.000228]  do_idle+0x1e8/0x280
[ 1141.000238]  cpu_startup_entry+0x40/0x50
[ 1141.000248]  rest_init+0x1c4/0x1d0
[ 1141.000260]  start_kernel+0x324/0x3e8
[ 1141.000272]  __primary_switched+0x88/0x98
[ 1141.197568] ------------[ cut here ]------------