[PATCH net] vxlan: vnifilter: Fix unlocked deletion of default FDB entry

[PATCH net] vxlan: vnifilter: Fix unlocked deletion of default FDB entry

[Ido Schimmel]

When a VNI is deleted from a VXLAN device in 'vnifilter' mode, the FDB
entry associated with the default remote (assuming one was configured)
is deleted without holding the hash lock. This is wrong and will result
in a warning [1] being generated by the lockdep annotation that was
added by commit ebe642067455 ("vxlan: Create wrappers for FDB lookup").

Reproducer:

 # ip link add vx0 up type vxlan dstport 4789 external vnifilter local 192.0.2.1
 # bridge vni add vni 10010 remote 198.51.100.1 dev vx0
 # bridge vni del vni 10010 dev vx0

Fix by acquiring the hash lock before the deletion and releasing it
afterwards. Blame the original commit that introduced the issue rather
than the one that exposed it.

[1]
WARNING: CPU: 3 PID: 392 at drivers/net/vxlan/vxlan_core.c:417 vxlan_find_mac+0x17f/0x1a0
[...]
RIP: 0010:vxlan_find_mac+0x17f/0x1a0
[...]
Call Trace:
 <TASK>
 __vxlan_fdb_delete+0xbe/0x560
 vxlan_vni_delete_group+0x2ba/0x940
 vxlan_vni_del.isra.0+0x15f/0x580
 vxlan_process_vni_filter+0x38b/0x7b0
 vxlan_vnifilter_process+0x3bb/0x510
 rtnetlink_rcv_msg+0x2f7/0xb70
 netlink_rcv_skb+0x131/0x360
 netlink_unicast+0x426/0x710
 netlink_sendmsg+0x75a/0xc20
 __sock_sendmsg+0xc1/0x150
 ____sys_sendmsg+0x5aa/0x7b0
 ___sys_sendmsg+0xfc/0x180
 __sys_sendmsg+0x121/0x1b0
 do_syscall_64+0xbb/0x1d0
 entry_SYSCALL_64_after_hwframe+0x4b/0x53

Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
I'm sorry, but I only noticed this issue after the recent VXLAN patches
were applied to net-next. There will be a conflict when merging net into
net-next, but resolution is trivial. Reference:
https://github.com/idosch/linux/commit/ed95370ec89cccbf784d5ef5ea4b6fb6fa0daf47.patch
---
 drivers/net/vxlan/vxlan_vnifilter.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index 6e6e9f05509a..06d19e90eadb 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -627,7 +627,11 @@ static void vxlan_vni_delete_group(struct vxlan_dev *vxlan,
 	 * default dst remote_ip previously added for this vni
 	 */
 	if (!vxlan_addr_any(&vninode->remote_ip) ||
-	    !vxlan_addr_any(&dst->remote_ip))
+	    !vxlan_addr_any(&dst->remote_ip)) {
+		u32 hash_index = fdb_head_index(vxlan, all_zeros_mac,
+						vninode->vni);
+
+		spin_lock_bh(&vxlan->hash_lock[hash_index]);
 		__vxlan_fdb_delete(vxlan, all_zeros_mac,
 				   (vxlan_addr_any(&vninode->remote_ip) ?
 				   dst->remote_ip : vninode->remote_ip),
@@ -635,6 +639,8 @@ static void vxlan_vni_delete_group(struct vxlan_dev *vxlan,
 				   vninode->vni, vninode->vni,
 				   dst->remote_ifindex,
 				   true);
+		spin_unlock_bh(&vxlan->hash_lock[hash_index]);
+	}
 
 	if (vxlan->dev->flags & IFF_UP) {
 		if (vxlan_addr_multicast(&vninode->remote_ip) &&
-- 
2.49.0

Re: [PATCH net] vxlan: vnifilter: Fix unlocked deletion of default FDB entry

[Nikolay Aleksandrov]

On 4/23/25 17:51, Ido Schimmel wrote:
> When a VNI is deleted from a VXLAN device in 'vnifilter' mode, the FDB
> entry associated with the default remote (assuming one was configured)
> is deleted without holding the hash lock. This is wrong and will result
> in a warning [1] being generated by the lockdep annotation that was
> added by commit ebe642067455 ("vxlan: Create wrappers for FDB lookup").
> 
> Reproducer:
> 
>  # ip link add vx0 up type vxlan dstport 4789 external vnifilter local 192.0.2.1
>  # bridge vni add vni 10010 remote 198.51.100.1 dev vx0
>  # bridge vni del vni 10010 dev vx0
> 
> Fix by acquiring the hash lock before the deletion and releasing it
> afterwards. Blame the original commit that introduced the issue rather
> than the one that exposed it.
> 
> [1]
> WARNING: CPU: 3 PID: 392 at drivers/net/vxlan/vxlan_core.c:417 vxlan_find_mac+0x17f/0x1a0
> [...]
> RIP: 0010:vxlan_find_mac+0x17f/0x1a0
> [...]
> Call Trace:
>  <TASK>
>  __vxlan_fdb_delete+0xbe/0x560
>  vxlan_vni_delete_group+0x2ba/0x940
>  vxlan_vni_del.isra.0+0x15f/0x580
>  vxlan_process_vni_filter+0x38b/0x7b0
>  vxlan_vnifilter_process+0x3bb/0x510
>  rtnetlink_rcv_msg+0x2f7/0xb70
>  netlink_rcv_skb+0x131/0x360
>  netlink_unicast+0x426/0x710
>  netlink_sendmsg+0x75a/0xc20
>  __sock_sendmsg+0xc1/0x150
>  ____sys_sendmsg+0x5aa/0x7b0
>  ___sys_sendmsg+0xfc/0x180
>  __sys_sendmsg+0x121/0x1b0
>  do_syscall_64+0xbb/0x1d0
>  entry_SYSCALL_64_after_hwframe+0x4b/0x53
> 
> Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
> I'm sorry, but I only noticed this issue after the recent VXLAN patches
> were applied to net-next. There will be a conflict when merging net into
> net-next, but resolution is trivial. Reference:
> https://github.com/idosch/linux/commit/ed95370ec89cccbf784d5ef5ea4b6fb6fa0daf47.patch
> ---
>  drivers/net/vxlan/vxlan_vnifilter.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Oops, yup. Thanks,
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH net] vxlan: vnifilter: Fix unlocked deletion of default FDB entry

[Jakub Kicinski]

On Wed, 23 Apr 2025 17:51:31 +0300 Ido Schimmel wrote:
> I'm sorry, but I only noticed this issue after the recent VXLAN patches
> were applied to net-next. There will be a conflict when merging net into
> net-next, but resolution is trivial. Reference:
> https://github.com/idosch/linux/commit/ed95370ec89cccbf784d5ef5ea4b6fb6fa0daf47.patch

Thanks! I guess this shouldn't happen often but FWIW for conflict-less
build breakage a patch on top of the merge would be more convenient
than the net-next version of the patch. Like this:

diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index 81d088c2f8dc..39b446a4bad7 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -628,7 +628,7 @@ static void vxlan_vni_delete_group(struct vxlan_dev *vxlan,
 		u32 hash_index = fdb_head_index(vxlan, all_zeros_mac,
 						vninode->vni);
 
-		spin_lock_bh(&vxlan->hash_lock[hash_index]);
+		spin_lock_bh(&vxlan->hash_lock);
 		__vxlan_fdb_delete(vxlan, all_zeros_mac,
 				   (vxlan_addr_any(&vninode->remote_ip) ?
 				   dst->remote_ip : vninode->remote_ip),
@@ -636,7 +636,7 @@ static void vxlan_vni_delete_group(struct vxlan_dev *vxlan,
 				   vninode->vni, vninode->vni,
 				   dst->remote_ifindex,
 				   true);
-		spin_unlock_bh(&vxlan->hash_lock[hash_index]);
+		spin_unlock_bh(&vxlan->hash_lock);
 	}
 
 	if (vxlan->dev->flags & IFF_UP) {
-- 
2.49.0

Re: [PATCH net] vxlan: vnifilter: Fix unlocked deletion of default FDB entry

[Ido Schimmel]

On Wed, Apr 23, 2025 at 02:29:21PM -0700, Jakub Kicinski wrote:
> On Wed, 23 Apr 2025 17:51:31 +0300 Ido Schimmel wrote:
> > I'm sorry, but I only noticed this issue after the recent VXLAN patches
> > were applied to net-next. There will be a conflict when merging net into
> > net-next, but resolution is trivial. Reference:
> > https://github.com/idosch/linux/commit/ed95370ec89cccbf784d5ef5ea4b6fb6fa0daf47.patch
> 
> Thanks! I guess this shouldn't happen often but FWIW for conflict-less
> build breakage a patch on top of the merge would be more convenient
> than the net-next version of the patch. Like this:

No problem, but note that "hash_index" needs to be removed as well, so
this would be the diff:

diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index 81d088c2f8dc..186d0660669a 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -625,10 +625,7 @@ static void vxlan_vni_delete_group(struct vxlan_dev *vxlan,
 	 */
 	if (!vxlan_addr_any(&vninode->remote_ip) ||
 	    !vxlan_addr_any(&dst->remote_ip)) {
-		u32 hash_index = fdb_head_index(vxlan, all_zeros_mac,
-						vninode->vni);
-
-		spin_lock_bh(&vxlan->hash_lock[hash_index]);
+		spin_lock_bh(&vxlan->hash_lock);
 		__vxlan_fdb_delete(vxlan, all_zeros_mac,
 				   (vxlan_addr_any(&vninode->remote_ip) ?
 				   dst->remote_ip : vninode->remote_ip),
@@ -636,7 +633,7 @@ static void vxlan_vni_delete_group(struct vxlan_dev *vxlan,
 				   vninode->vni, vninode->vni,
 				   dst->remote_ifindex,
 				   true);
-		spin_unlock_bh(&vxlan->hash_lock[hash_index]);
+		spin_unlock_bh(&vxlan->hash_lock);
 	}
 
 	if (vxlan->dev->flags & IFF_UP) {

Re: [PATCH net] vxlan: vnifilter: Fix unlocked deletion of default FDB entry

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 23 Apr 2025 17:51:31 +0300 you wrote:
> When a VNI is deleted from a VXLAN device in 'vnifilter' mode, the FDB
> entry associated with the default remote (assuming one was configured)
> is deleted without holding the hash lock. This is wrong and will result
> in a warning [1] being generated by the lockdep annotation that was
> added by commit ebe642067455 ("vxlan: Create wrappers for FDB lookup").
> 
> Reproducer:
> 
> [...]

Here is the summary with links:
  - [net] vxlan: vnifilter: Fix unlocked deletion of default FDB entry
    https://git.kernel.org/netdev/net/c/087a9eb9e597

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html