[PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy

[PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy

[Ali Firas]

VXLAN_VNIFILTER_ENTRY_START and VXLAN_VNIFILTER_ENTRY_END are declared as
bare NLA_U32, so neither is range-checked before vxlan_process_vni_filter()
passes them to vxlan_vni_add_del():

	int v, err = 0;

	for (v = start_vni; v <= end_vni; v++) {

v is int and end_vni is __u32, so the comparison is unsigned. With
end_vni == U32_MAX the loop cannot terminate through its own condition:
v reaches U32_MAX, wraps to 0, and 0 <= U32_MAX is true again, so the
request never returns. It runs under rtnl_lock, which is global rather
than per-netns, so every network configuration operation on the host
blocks for as long as it runs, in every namespace.

The interface is reachable without privilege: creating the device and
adding VNIs only requires CAP_NET_ADMIN in the network namespace's user
namespace, so an unprivileged user inside unshare(CLONE_NEWUSER |
CLONE_NEWNET) can trigger this with a single netlink message. A VNI at or
above VXLAN_N_VID is also accepted and stored, although the VXLAN header
carries only 24 bits.

The MDB interface in the same driver already range-validates its VNI
attributes with an identical constraint (vxlan_mdb.c, vni_range with
.max = VXLAN_N_VID - 1). Apply the same validation here.

This removes the non-terminating case and rejects VNIs the header cannot
carry. It does not bound the cost of a request spanning the whole
legitimate 24-bit space: that still creates 2^24 nodes, each with a
per-CPU stats block, under rtnl_lock and with no reschedule point, and
neither allocation carries __GFP_ACCOUNT. Bounding or accounting that is
a separate change and is not attempted here.

Tested in a QEMU guest on a KASAN kernel with 2G of memory, as an
unprivileged uid inside unshare(CLONE_NEWUSER | CLONE_NEWNET). Before the
change, a request with START=0 and END=0xFFFFFFFF drives a global OOM
with the allocating task in vxlan_vnifilter_process(); after it, the same
request is rejected and in-range VNI addition is unaffected. A request
spanning the full in-range space, START=0 END=0xFFFFFF, still exhausts
memory on that guest both before and after, as described above.

Reproducer available on request.

Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Ali Firas <alishmery18@gmail.com>
---
v2: rewrite the changelog to describe only what the patch closes and
    state explicitly that the cost of a full in-range request is not
    bounded here. No code changes.
    v1: https://lore.kernel.org/netdev/20260829030041.940594-1-alishmery18@gmail.com/

 drivers/net/vxlan/vxlan_vnifilter.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
index dd94085e0886..9e86ac39cf9d 100644
--- a/drivers/net/vxlan/vxlan_vnifilter.c
+++ b/drivers/net/vxlan/vxlan_vnifilter.c
@@ -459,9 +459,15 @@ static int vxlan_vnifilter_dump(struct sk_buff *skb, struct netlink_callback *cb
 	return err;
 }
 
+static const struct netlink_range_validation vni_filter_vni_range = {
+	.max = VXLAN_N_VID - 1,
+};
+
 static const struct nla_policy vni_filter_entry_policy[VXLAN_VNIFILTER_ENTRY_MAX + 1] = {
-	[VXLAN_VNIFILTER_ENTRY_START] = { .type = NLA_U32 },
-	[VXLAN_VNIFILTER_ENTRY_END] = { .type = NLA_U32 },
+	[VXLAN_VNIFILTER_ENTRY_START] = NLA_POLICY_FULL_RANGE(NLA_U32,
+							      &vni_filter_vni_range),
+	[VXLAN_VNIFILTER_ENTRY_END] = NLA_POLICY_FULL_RANGE(NLA_U32,
+							    &vni_filter_vni_range),
 	[VXLAN_VNIFILTER_ENTRY_GROUP]	= NLA_POLICY_EXACT_LEN(sizeof_field(struct iphdr, daddr)),
 	[VXLAN_VNIFILTER_ENTRY_GROUP6]	= NLA_POLICY_EXACT_LEN(sizeof(struct in6_addr)),
 };
-- 
2.53.0

Re: [PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] Incomplete fix relative to the impact its own changelog
  describes: after the range clamp to VXLAN_N_VID-1, a single…

--

vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy

This adds an NLA_POLICY_FULL_RANGE constraint with .max = VXLAN_N_VID - 1
to VXLAN_VNIFILTER_ENTRY_START and VXLAN_VNIFILTER_ENTRY_END, matching
what vxlan_mdb.c already does for its VNI attributes.  The commit carries
Fixes: f9c4bb0b245c and Cc: stable@vger.kernel.org.

No Link: tags in the commit header; the v1 posting is referenced in the
patch notes as
https://lore.kernel.org/netdev/20260829030041.940594-1-alishmery18@gmail.com/

> diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c
> index dd94085e0886..9e86ac39cf9d 100644
> --- a/drivers/net/vxlan/vxlan_vnifilter.c
> +++ b/drivers/net/vxlan/vxlan_vnifilter.c
> @@ -459,9 +459,15 @@ static int vxlan_vnifilter_dump(struct sk_buff *skb, struct netlink_callback *cb
>  	return err;
>  }
>  
> +static const struct netlink_range_validation vni_filter_vni_range = {
> +	.max = VXLAN_N_VID - 1,
> +};
> +
>  static const struct nla_policy vni_filter_entry_policy[VXLAN_VNIFILTER_ENTRY_MAX + 1] = {
> -	[VXLAN_VNIFILTER_ENTRY_START] = { .type = NLA_U32 },
> -	[VXLAN_VNIFILTER_ENTRY_END] = { .type = NLA_U32 },
> +	[VXLAN_VNIFILTER_ENTRY_START] = NLA_POLICY_FULL_RANGE(NLA_U32,
> +							      &vni_filter_vni_range),
> +	[VXLAN_VNIFILTER_ENTRY_END] = NLA_POLICY_FULL_RANGE(NLA_U32,
> +							    &vni_filter_vni_range),

[Severity: High]
Does this clamp close the problem the changelog describes, or only the
U32_MAX endpoint?

With the new .max, a single PF_BRIDGE RTM_NEWTUNNEL message carrying only
VXLAN_VNIFILTER_ENTRY_END = 0xFFFFFF still passes validation, because
vni_start defaults to 0 in vxlan_process_vni_filter():

	u32 vni_start = 0, vni_end = 0;
	...
	if (vattrs[VXLAN_VNIFILTER_ENTRY_START]) {
		vni_start = nla_get_u32(vattrs[VXLAN_VNIFILTER_ENTRY_START]);
		vni_end = vni_start;
	}

	if (vattrs[VXLAN_VNIFILTER_ENTRY_END])
		vni_end = nla_get_u32(vattrs[VXLAN_VNIFILTER_ENTRY_END]);

	if (!vni_start && !vni_end) {
		NL_SET_ERR_MSG_ATTR(extack, nlvnifilter,
				    "vni start nor end found in vni entry");
		return -EINVAL;
	}

vni_end is non-zero so the guard does not fire, and vxlan_vni_add_del() is
entered with the full 0..0xFFFFFF span.

Each of those 2^24 iterations goes through vxlan_vni_add() ->
vxlan_vni_alloc(), which allocates a node plus a full per-CPU stats block:

	vninode = kzalloc_obj(*vninode);
	if (!vninode)
		return NULL;
	vninode->stats = netdev_alloc_pcpu_stats(struct vxlan_vni_stats_pcpu);

followed by an rhashtable insert, a sorted list insert and a
vxlan_vnifilter_notify().  Neither allocation uses __GFP_ACCOUNT, so the
memory is charged globally rather than to the requesting cgroup.

There is also no cond_resched() in vxlan_vni_add_del() (grep finds none in
the file), no per-request work cap and no ceiling on vg->num_vnis, and
vxlan_vnifilter_rtnl_msg_handlers registers the handler without
RTNL_FLAG_DOIT_UNLOCKED, so the whole span runs under the global rtnl_lock.

Given that, is the Fixes: plus Cc: stable framing accurate?  The changelog
states:

	It runs under rtnl_lock, which is global rather than per-netns, so
	every network configuration operation on the host blocks for as long
	as it runs, in every namespace.

	The interface is reachable without privilege: creating the device and
	adding VNIs only requires CAP_NET_ADMIN in the network namespace's
	user namespace

and then:

	A request spanning the full in-range space, START=0 END=0xFFFFFF,
	still exhausts memory on that guest both before and after, as
	described above.

Would a backporter reading the first part conclude that the rtnl_lock
starvation and unprivileged memory exhaustion are closed, when the second
part says they are not?

Separately, is this description of the U32_MAX case right?

	With end_vni == U32_MAX the loop cannot terminate through its own
	condition: v reaches U32_MAX, wraps to 0, and 0 <= U32_MAX is true
	again, so the request never returns.

The test result in the same changelog says that request "drives a global OOM
with the allocating task in vxlan_vnifilter_process()".  Doesn't the loop
actually leave early once vxlan_vni_alloc() fails?

	err = vxlan_vni_add(vxlan, vg, v, group, extack);
	...
	if (err)
		goto out;

so the observed exit is -ENOMEM rather than a loop that never returns.

One more question on the partial-failure path: when vxlan_vni_add() fails
part way through a range, nothing unwinds the VNIs already added, and they
stay in vg->vni_hash and vg->vni_list until vxlan_vnigroup_uninit().  Is a
partially applied range the intended behaviour here?

Would it make sense to bound the per-request work in the same change, for
example capping the number of VNIs one message may span, adding a reschedule
point to the vxlan_vni_add_del() loop, and using __GFP_ACCOUNT for the node
and stats allocations, so that the tags match what the patch removes?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902154609.594009-1-alishmery18%40gmail.com

Re: [PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy

[Jakub Kicinski]

On Sat, 05 Sep 2026 19:36:51 +0000 netdev-bot+sashiko@kernel.org wrote:
> > +	[VXLAN_VNIFILTER_ENTRY_START] = NLA_POLICY_FULL_RANGE(NLA_U32,
> > +							      &vni_filter_vni_range),
> > +	[VXLAN_VNIFILTER_ENTRY_END] = NLA_POLICY_FULL_RANGE(NLA_U32,
> > +							    &vni_filter_vni_range),  
> 
> [Severity: High]
> Does this clamp close the problem the changelog describes, or only the
> U32_MAX endpoint?

I tend to agree, either this is a security fix and we need a stronger
check. Or it's just making things slightly better and doesn't deserve
Fixes/stable.
-- 
pw-bot: cr