mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v2] vxlan: mdb: Fix use-after-free in vxlan_mdb_remote_src_del()
@ 2026-08-26 17:36 Baul Lee
  2026-08-26 17:40 ` Nikolay Aleksandrov
  2026-08-27  9:17 ` Ido Schimmel
  0 siblings, 2 replies; 3+ messages in thread
From: Baul Lee @ 2026-08-26 17:36 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Baul Lee, Ido Schimmel, Nikolay Aleksandrov, netdev,
	linux-kernel, federico.kirschbaum

vxlan_mdb_is_valid_source(), which validates MDBE_ATTR_SOURCE and every
MDBE_ATTR_SRC_LIST member, accepts the all-zeros address.

A source list is only accepted on a (*, G) entry, whose source is the
all-zeros address, and for each member of the list an (S, G) entry is
derived from it by substituting the source. Entries are keyed by a plain
memcmp() of struct vxlan_mdb_entry_key, so if MDBE_ATTR_SOURCE is present
and holds the all-zeros address and the source list holds it as well, the
derived (S, G) key is byte-identical to the (*, G) key and resolves to the
same entry. Omitting MDBE_ATTR_SOURCE is not equivalent, as the key is
then left with a zero address family.

vxlan_mdb_remote_src_del() removes the forwarding entry of a source before
freeing the source entry:

	vxlan_mdb_remote_src_fwd_del(vxlan, group, remote, &ent->addr);
	vxlan_mdb_remote_src_entry_del(ent);

With the keys aliased, the first call deletes the remote of the entry that
owns 'ent' instead of a separate (S, G) entry, and frees 'ent'. The second
call then runs on the freed entry, and its hlist_del() reads ->pprev and
->next out of it and writes through them.

Adding the (*, G) entry with NLM_F_REPLACE and no source list marks the
all-zeros source for deletion and reaches this from the sweep at the end
of vxlan_mdb_remote_srcs_replace().

  BUG: KASAN: slab-use-after-free in __vxlan_mdb_add+0x1cd/0xd70
  Read of size 8 at addr ffff888102852500 by task poc/84
   __vxlan_mdb_add+0x1cd/0xd70
   vxlan_mdb_add+0xc0/0x140
   rtnl_mdb_add+0x157/0x2a0
   rtnetlink_rcv_msg+0x207/0x5a0
  Allocated by task 84:
   __kmalloc_cache_noprof+0x153/0x360
   vxlan_mdb_remote_srcs_add+0x2eb/0x440
   __vxlan_mdb_add+0x803/0xd70
  Freed by task 84:
   kfree+0x14c/0x3b0
   vxlan_mdb_remote_del+0x129/0x1a0
   __vxlan_mdb_del+0x4f/0xe0
   vxlan_mdb_remote_src_fwd_del.isra.0+0x162/0x1b0
   __vxlan_mdb_add+0x1c5/0xd70

The MDB operations are netns-scoped, so an unprivileged user can perform
them in a new user and network namespace.

Reject the all-zeros address in vxlan_mdb_is_valid_source(), which covers
both call sites. A (*, G) entry is expressed by omitting the source, so
nothing legitimate is refused.

Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>

Fixes: a3a48de5eade ("vxlan: mdb: Add MDB control path support")
Signed-off-by: Baul Lee <baul.lee@xbow.com>
---
Unrelated to [1]: different subsystem, different bug class, different fix
site. It applies independently.

v2: Use ipv4_is_zeronet(). Add test cases to star_g_common() and
    sg_common().

v1: https://lore.kernel.org/netdev/20260826151542.68815-1-baul.lee@xbow.com/
[1] https://lore.kernel.org/netdev/20260815172032.79740-1-baul.lee@xbow.com/

 drivers/net/vxlan/vxlan_mdb.c                 | 8 ++++++++
 tools/testing/selftests/net/test_vxlan_mdb.sh | 6 ++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/net/vxlan/vxlan_mdb.c b/drivers/net/vxlan/vxlan_mdb.c
index d71e1925ecfd..841f42ffecb9 100644
--- a/drivers/net/vxlan/vxlan_mdb.c
+++ b/drivers/net/vxlan/vxlan_mdb.c
@@ -354,6 +354,10 @@ static bool vxlan_mdb_is_valid_source(const struct nlattr *attr, __be16 proto,
 			NL_SET_ERR_MSG_MOD(extack, "IPv4 multicast source address is not allowed");
 			return false;
 		}
+		if (ipv4_is_zeronet(nla_get_in_addr(attr))) {
+			NL_SET_ERR_MSG_MOD(extack, "IPv4 all-zeros source address is not allowed");
+			return false;
+		}
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case htons(ETH_P_IPV6): {
@@ -368,6 +372,10 @@ static bool vxlan_mdb_is_valid_source(const struct nlattr *attr, __be16 proto,
 			NL_SET_ERR_MSG_MOD(extack, "IPv6 multicast source address is not allowed");
 			return false;
 		}
+		if (ipv6_addr_any(&src)) {
+			NL_SET_ERR_MSG_MOD(extack, "IPv6 all-zeros source address is not allowed");
+			return false;
+		}
 		break;
 	}
 #endif
diff --git a/tools/testing/selftests/net/test_vxlan_mdb.sh b/tools/testing/selftests/net/test_vxlan_mdb.sh
index 58da5de99ac4..f9600aabd4a2 100755
--- a/tools/testing/selftests/net/test_vxlan_mdb.sh
+++ b/tools/testing/selftests/net/test_vxlan_mdb.sh
@@ -685,6 +685,9 @@ star_g_common()
 	run_cmd "bridge -n $ns1 mdb add dev vx0 port vx0 grp $grp permanent filter_mode exclude source_list $grp dst $vtep_ip src_vni 10010"
 	log_test $? 255 "Invalid source in source list"
 
+	run_cmd "bridge -n $ns1 mdb add dev vx0 port vx0 grp $grp permanent filter_mode exclude source_list $all_zeros_grp dst $vtep_ip src_vni 10010"
+	log_test $? 255 "All-zeros source in source list"
+
 	run_cmd "bridge -n $ns1 mdb add dev vx0 port vx0 grp $grp permanent source_list $src1 dst $vtep_ip src_vni 10010"
 	log_test $? 255 "Source list without filter mode"
 }
@@ -784,6 +787,9 @@ sg_common()
 	run_cmd "bridge -n $ns1 mdb add dev vx0 port vx0 grp $grp src $grp permanent dst $vtep_ip src_vni 10010"
 	log_test $? 255 "(S, G) with an invalid source list"
 
+	run_cmd "bridge -n $ns1 mdb add dev vx0 port vx0 grp $grp src $all_zeros_grp permanent dst $vtep_ip src_vni 10010"
+	log_test $? 255 "(S, G) with an all-zeros source"
+
 	run_cmd "bridge -n $ns1 mdb add dev vx0 port vx0 grp $all_zeros_grp src $src permanent dst $vtep_ip src_vni 10010"
 	log_test $? 255 "All-zeros group with source"
 }

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] vxlan: mdb: Fix use-after-free in vxlan_mdb_remote_src_del()
  2026-08-26 17:36 [PATCH net v2] vxlan: mdb: Fix use-after-free in vxlan_mdb_remote_src_del() Baul Lee
@ 2026-08-26 17:40 ` Nikolay Aleksandrov
  2026-08-27  9:17 ` Ido Schimmel
  1 sibling, 0 replies; 3+ messages in thread
From: Nikolay Aleksandrov @ 2026-08-26 17:40 UTC (permalink / raw)
  To: Baul Lee, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: Ido Schimmel, netdev, linux-kernel, federico.kirschbaum

On 26/08/2026 20:36, Baul Lee wrote:
> vxlan_mdb_is_valid_source(), which validates MDBE_ATTR_SOURCE and every
> MDBE_ATTR_SRC_LIST member, accepts the all-zeros address.
> 
> A source list is only accepted on a (*, G) entry, whose source is the
> all-zeros address, and for each member of the list an (S, G) entry is
> derived from it by substituting the source. Entries are keyed by a plain
> memcmp() of struct vxlan_mdb_entry_key, so if MDBE_ATTR_SOURCE is present
> and holds the all-zeros address and the source list holds it as well, the
> derived (S, G) key is byte-identical to the (*, G) key and resolves to the
> same entry. Omitting MDBE_ATTR_SOURCE is not equivalent, as the key is
> then left with a zero address family.
> 
> vxlan_mdb_remote_src_del() removes the forwarding entry of a source before
> freeing the source entry:
> 
> 	vxlan_mdb_remote_src_fwd_del(vxlan, group, remote, &ent->addr);
> 	vxlan_mdb_remote_src_entry_del(ent);
> 
> With the keys aliased, the first call deletes the remote of the entry that
> owns 'ent' instead of a separate (S, G) entry, and frees 'ent'. The second
> call then runs on the freed entry, and its hlist_del() reads ->pprev and
> ->next out of it and writes through them.
> 
> Adding the (*, G) entry with NLM_F_REPLACE and no source list marks the
> all-zeros source for deletion and reaches this from the sweep at the end
> of vxlan_mdb_remote_srcs_replace().
> 
>    BUG: KASAN: slab-use-after-free in __vxlan_mdb_add+0x1cd/0xd70
>    Read of size 8 at addr ffff888102852500 by task poc/84
>     __vxlan_mdb_add+0x1cd/0xd70
>     vxlan_mdb_add+0xc0/0x140
>     rtnl_mdb_add+0x157/0x2a0
>     rtnetlink_rcv_msg+0x207/0x5a0
>    Allocated by task 84:
>     __kmalloc_cache_noprof+0x153/0x360
>     vxlan_mdb_remote_srcs_add+0x2eb/0x440
>     __vxlan_mdb_add+0x803/0xd70
>    Freed by task 84:
>     kfree+0x14c/0x3b0
>     vxlan_mdb_remote_del+0x129/0x1a0
>     __vxlan_mdb_del+0x4f/0xe0
>     vxlan_mdb_remote_src_fwd_del.isra.0+0x162/0x1b0
>     __vxlan_mdb_add+0x1c5/0xd70
> 
> The MDB operations are netns-scoped, so an unprivileged user can perform
> them in a new user and network namespace.
> 
> Reject the all-zeros address in vxlan_mdb_is_valid_source(), which covers
> both call sites. A (*, G) entry is expressed by omitting the source, so
> nothing legitimate is refused.
> 
> Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>
> 
> Fixes: a3a48de5eade ("vxlan: mdb: Add MDB control path support")
> Signed-off-by: Baul Lee <baul.lee@xbow.com>
> ---

You should wait 24 hours before posting another version

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] vxlan: mdb: Fix use-after-free in vxlan_mdb_remote_src_del()
  2026-08-26 17:36 [PATCH net v2] vxlan: mdb: Fix use-after-free in vxlan_mdb_remote_src_del() Baul Lee
  2026-08-26 17:40 ` Nikolay Aleksandrov
@ 2026-08-27  9:17 ` Ido Schimmel
  1 sibling, 0 replies; 3+ messages in thread
From: Ido Schimmel @ 2026-08-27  9:17 UTC (permalink / raw)
  To: Baul Lee
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Nikolay Aleksandrov, netdev, linux-kernel,
	federico.kirschbaum

On Thu, Aug 27, 2026 at 02:36:04AM +0900, Baul Lee wrote:
> vxlan_mdb_is_valid_source(), which validates MDBE_ATTR_SOURCE and every
> MDBE_ATTR_SRC_LIST member, accepts the all-zeros address.
> 
> A source list is only accepted on a (*, G) entry, whose source is the
> all-zeros address, and for each member of the list an (S, G) entry is
> derived from it by substituting the source. Entries are keyed by a plain
> memcmp() of struct vxlan_mdb_entry_key, so if MDBE_ATTR_SOURCE is present
> and holds the all-zeros address and the source list holds it as well, the
> derived (S, G) key is byte-identical to the (*, G) key and resolves to the
> same entry. Omitting MDBE_ATTR_SOURCE is not equivalent, as the key is
> then left with a zero address family.
> 
> vxlan_mdb_remote_src_del() removes the forwarding entry of a source before
> freeing the source entry:
> 
> 	vxlan_mdb_remote_src_fwd_del(vxlan, group, remote, &ent->addr);
> 	vxlan_mdb_remote_src_entry_del(ent);
> 
> With the keys aliased, the first call deletes the remote of the entry that
> owns 'ent' instead of a separate (S, G) entry, and frees 'ent'. The second
> call then runs on the freed entry, and its hlist_del() reads ->pprev and
> ->next out of it and writes through them.
> 
> Adding the (*, G) entry with NLM_F_REPLACE and no source list marks the
> all-zeros source for deletion and reaches this from the sweep at the end
> of vxlan_mdb_remote_srcs_replace().

[...]

> The MDB operations are netns-scoped, so an unprivileged user can perform
> them in a new user and network namespace.
> 
> Reject the all-zeros address in vxlan_mdb_is_valid_source(), which covers
> both call sites. A (*, G) entry is expressed by omitting the source, so
> nothing legitimate is refused.
> 
> Discovered by XBOW, triaged by Baul Lee <baul.lee@xbow.com>
> 
> Fixes: a3a48de5eade ("vxlan: mdb: Add MDB control path support")
> Signed-off-by: Baul Lee <baul.lee@xbow.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-27  9:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-26 17:36 [PATCH net v2] vxlan: mdb: Fix use-after-free in vxlan_mdb_remote_src_del() Baul Lee
2026-08-26 17:40 ` Nikolay Aleksandrov
2026-08-27  9:17 ` Ido Schimmel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®