* [PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy
@ 2026-09-02 15:46 Ali Firas
2026-09-05 19:36 ` netdev-bot+sashiko
2026-09-06 23:16 ` Ali Firas
0 siblings, 2 replies; 13+ messages in thread
From: Ali Firas @ 2026-09-02 15:46 UTC (permalink / raw)
To: netdev
Cc: kuba, pabeni, davem, edumazet, andrew+netdev, idosch, razor,
stable, linux-kernel, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy 2026-09-02 15:46 [PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy Ali Firas @ 2026-09-05 19:36 ` netdev-bot+sashiko 2026-09-05 19:39 ` Jakub Kicinski 2026-09-06 23:16 ` Ali Firas 1 sibling, 1 reply; 13+ messages in thread From: netdev-bot+sashiko @ 2026-09-05 19:36 UTC (permalink / raw) To: alishmery18 Cc: netdev, kuba, pabeni, davem, edumazet, andrew+netdev, idosch, razor, stable, linux-kernel 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy 2026-09-05 19:36 ` netdev-bot+sashiko @ 2026-09-05 19:39 ` Jakub Kicinski 0 siblings, 0 replies; 13+ messages in thread From: Jakub Kicinski @ 2026-09-05 19:39 UTC (permalink / raw) To: netdev-bot+sashiko Cc: alishmery18, netdev, pabeni, davem, edumazet, andrew+netdev, idosch, razor, stable, linux-kernel 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy 2026-09-02 15:46 [PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy Ali Firas 2026-09-05 19:36 ` netdev-bot+sashiko @ 2026-09-06 23:16 ` Ali Firas 2026-09-07 10:52 ` Ali Firas 1 sibling, 1 reply; 13+ messages in thread From: Ali Firas @ 2026-09-06 23:16 UTC (permalink / raw) To: kuba; +Cc: netdev, idosch, pabeni, davem, edumazet, andrew+netdev, Ali Firas > 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. You're right, and my justification was worse than incomplete. Sorry for the slow reply. To be precise about what is and isn't real: the loop's own bound genuinely is broken at END=U32_MAX -- v is int, end_vni is __u32, so the comparison is unsigned and the loop has no exit condition of its own; it leaves only via "if (err) goto out". But the wrap is unreachable. vxlan_vni_add() allocates for every fresh VNI, so -ENOMEM ends the loop long before v could wrap, and the delete path stops at the first absent VNI with -ENOENT. So it is not a non-terminating loop, and the changelog should never have said so. What I did observe in a 2G KASAN guest was a global OOM, and that is a resource problem, not a control-flow one. The clamp does not address it: START=0 END=0xFFFFFF passes the new policy and allocates just the same. vxlan_vni_alloc() uses plain GFP_KERNEL for both the node and its per-CPU stats, so none of it is charged to the caller. By size -- I haven't measured the struct yet -- that is on the order of 192 bytes plus 64 per CPU per VNI, so a full in-range request is a few GB on 2 CPUs and tens of GB on 64, from one netlink message, by an unprivileged user holding CAP_NET_ADMIN in a netns. That looks like the same class as 1beb81947eb4 ("net/sched: account classifier filter allocations to memcg"), and __netdev_alloc_pcpu_stats() already takes a gfp, so no new API is needed. One difference worth flagging: that patch also had to fix a real error-path hazard, because cls_basic did idr_alloc before alloc_percpu. vxlan doesn't -- both allocations are inside vxlan_vni_alloc() and return NULL before rhashtable_lookup_insert_fast(), so making them failable exposes nothing new. So I'd rather split this: - GFP_KERNEL_ACCOUNT on the node and its per-CPU stats. This is the actual fix. Given the difference above, I'm not sure whether it belongs in net with a Fixes tag or in net-next -- happy to go either way. - the range validation on its own, to net-next, no Fixes and no security claim. Its only real justification is that vxlan_vni_rht_params already declares .max_size = VXLAN_N_VID, so the netlink edge never enforced the bound the driver assumes. It does tighten uAPI: requests that used to succeed with an out-of-range VNI will now get -EINVAL. I'm measuring the full in-range request against the accounting patch before sending anything. Thanks for the review, Ali ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy 2026-09-06 23:16 ` Ali Firas @ 2026-09-07 10:52 ` Ali Firas 2026-09-07 14:10 ` Ido Schimmel 0 siblings, 1 reply; 13+ messages in thread From: Ali Firas @ 2026-09-07 10:52 UTC (permalink / raw) To: kuba; +Cc: netdev, idosch, pabeni, davem, edumazet, andrew+netdev, Ali Firas I have the measurements now. They split into two independent problems, which changes what I was going to send. Cost per VNI, measured with per-cache attribution rather than estimated: 128 B slab (kmalloc-128, exactly 1.000 objects per VNI) plus 64 B per possible CPU for the stats block. rhashtable buckets add roughly 11-21 B amortised; the RTM_NEWTUNNEL skb is freed promptly and retains nothing. So 256 B/VNI on 2 CPUs, and a full in-range request is about 66 GiB on a 64-CPU host. My earlier 521 B figure was an artifact of CONFIG_SLUB_DEBUG_ON inflating the object to 384 B; it is withdrawn. rhashtable cannot intervene: max_elems is 2^25 against a 2^24 reachable key space, so -E2BIG is structurally unreachable. Every configuration I tested (2/4/8 CPUs) reaches a global OOM instead, with no errno returned because the caller is OOM-killed, and UID 0 processes killed in most runs. First problem, memory. GFP_KERNEL_ACCOUNT on the node and its per-CPU stats confines this: the OOM becomes CONSTRAINT_MEMCG, the host survives, and unconstrained callers are unaffected within 0.4% on both VNI count and wall time. It does not produce a graceful failure — try_charge() invokes the memcg OOM killer rather than returning -ENOMEM, and the partially installed VNIs stay until the netns is torn down. Second problem, and this is the one I had not measured when I wrote last time. vxlan_vnifilter_rtnl_msg_handlers registers RTM_NEWTUNNEL with flags = 0, so rtnetlink takes the locked path and rtnl_lock is held across the whole loop. From a second namespace, "ip link add dummy0 type dummy" takes 0.011 s normally, 4.47 s during a 1M-VNI request, and never completes at all during a full-range request — it is still blocked when the OOM killer arrives. rtnl_lock is global rather than per-netns, so an unprivileged user in one namespace stalls network configuration for the host and every other namespace. memcg accounting does nothing for that. cond_resched() yields the CPU without releasing rtnl, so it only addresses soft-lockup warnings. As far as I can see only a per-request work cap would fix it, and that would change uAPI since the full 24-bit space is a legitimate request today. So I have the accounting patch ready, but I would rather not send it as if it closed the problem when it closes half of it. Would you prefer the accounting patch on its own with the rtnl stall described as a known remaining issue, or is the stall something you would want addressed first, in which case I would need guidance on whether a cap is acceptable at all? Reproducer and full measurement data available on request. Thanks, Ali ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy 2026-09-07 10:52 ` Ali Firas @ 2026-09-07 14:10 ` Ido Schimmel 2026-09-09 9:26 ` [PATCH net 0/3] vxlan: vnifilter: bound the VNI range per request Ali Firas 0 siblings, 1 reply; 13+ messages in thread From: Ido Schimmel @ 2026-09-07 14:10 UTC (permalink / raw) To: Ali Firas; +Cc: kuba, netdev, pabeni, davem, edumazet, andrew+netdev On Mon, Sep 07, 2026 at 01:52:23PM +0300, Ali Firas wrote: > So I have the accounting patch ready, but I would rather not send it as > if it closed the problem when it closes half of it. Would you prefer the > accounting patch on its own with the rtnl stall described as a known > remaining issue, or is the stall something you would want addressed > first, in which case I would need guidance on whether a cap is > acceptable at all? Limiting the VNI range to 4k at a time is unlikely to break anyone given that "vnifilter" is mainly used on bridged VXLAN devices where the VNI is derived from the VLAN (max 4094). There are no selftests for the range functionality, so as part of this work please add some in test_vxlan_vnifiltering.sh. At the very least we should make sure that the max range is accepted and 'max + 1' is rejected (for both add and delete). ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net 0/3] vxlan: vnifilter: bound the VNI range per request 2026-09-07 14:10 ` Ido Schimmel @ 2026-09-09 9:26 ` Ali Firas 2026-09-09 9:26 ` [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request Ali Firas ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Ali Firas @ 2026-09-09 9:26 UTC (permalink / raw) To: netdev, idosch Cc: kuba, pabeni, davem, edumazet, andrew+netdev, razor, roopa, linux-kernel, Ali Firas A single RTM_NEWTUNNEL or RTM_DELTUNNEL message can currently ask the vnifilter code for the whole 24-bit VNI space, and vxlan_vni_add_del() loops over that span under rtnl_lock creating one VNI node and one per-CPU stats block per iteration. That is reachable by an unprivileged user in a user+network namespace and it costs roughly 2.1 GiB + 1 GiB per possible CPU (128 B slab plus 64 B per possible CPU per VNI): on 2, 4 and 8 CPU guests every full-range request ended in a global OOM, with no errno returned because the caller is itself OOM-killed, and with unrelated root-owned processes killed on the way down. Because rtnl_lock is global rather than per-netns, it also stalled network configuration everywhere: a plain "ip link add dummy0 type dummy" in a different network namespace takes 0.011 s normally, 4.472 s while a 1,000,000 VNI request runs, and never completed at all during a full-range one. Patch 1 caps the span of a single request at 4096 VNIs, which closes both. The cap is on one request, not on the device: a device can still hold the whole VNI space, it just takes more than one message. Patch 2 charges the VNI node and its per-CPU stats to the caller's memcg. With the cap in place this is no longer the primary defence, but nothing limits how many capped requests a task may issue, so an unprivileged user can still accumulate VNIs 4096 at a time with none of it charged to them. It is also the same class fix as commit 1beb81947eb4 ("net/sched: account classifier filter allocations to memcg"). One limitation is worth stating up front: try_charge() reclaims and then invokes the memcg OOM killer rather than returning -ENOMEM, so accounting confines the blast radius without producing a graceful failure. Patch 3 adds the selftest coverage Ido asked for, in the existing API test: a range of exactly the maximum is accepted and one VNI more is rejected, for both add and delete. The limit is a driver-local constant rather than VLAN_N_VID: the values coincide today, but a bound on a VXLAN netlink request is not a count of VLAN IDs, and coupling them would make a change to one silently change the other. Patch 1 does tighten uAPI: a request spanning more than 4096 VNIs used to succeed and now returns -EINVAL. Ido's assessment was that the limit is unlikely to break anyone, since vnifilter is mainly used on bridged VXLAN devices where the VNI is derived from the VLAN, capped at 4094. I am sending the series to net because the stall is reachable by an unprivileged user and crosses namespaces. What I am less sure about is that Fixes: in net means this reaches stable, where a script issuing one large range would start failing across a point release. If that is the wrong trade, I am happy to respin patch 1 against net-next without the Fixes tag. Two things the series does not address. A single-VNI request with START == END >= VXLAN_N_VID still passes the span check, and vxlan_vni_field() shifts without masking, so such an entry is silently truncated on the wire while holding its own rhashtable slot. That wants a netlink policy range check, which I will send separately to net-next as a pure uAPI tightening. Measured worst case at the cap, on a 2 CPU / 2G guest: a request of exactly 4096 VNIs takes 0.031 s to add and 0.022 s to delete, and the cross-namespace "ip link add" blocks for 0.024 s during it. The old full-range request is now rejected outright with Error: VNI range spans more than 4096 VNIs. for both add and delete. The whole selftest file passes before and after: 27 tests passed and 0 failed on the base, 31 passed and 0 failed with the series applied. The 4096 limit follows Ido Schimmel's suggestion: https://lore.kernel.org/netdev/20260907141001.GA708129@shredder/ Ali Firas (3): vxlan: vnifilter: limit the VNI range of a single request vxlan: vnifilter: account VNI node and per-CPU stats to memcg selftests: net: test the vxlan vnifilter VNI range limit drivers/net/vxlan/vxlan_vnifilter.c | 24 +++++++++++++++++-- .../selftests/net/test_vxlan_vnifiltering.sh | 13 ++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) -- 2.53.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request 2026-09-09 9:26 ` [PATCH net 0/3] vxlan: vnifilter: bound the VNI range per request Ali Firas @ 2026-09-09 9:26 ` Ali Firas 2026-09-10 9:38 ` netdev-bot+sashiko 2026-09-09 9:26 ` [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg Ali Firas 2026-09-09 9:26 ` [PATCH net 3/3] selftests: net: test the vxlan vnifilter VNI range limit Ali Firas 2 siblings, 1 reply; 13+ messages in thread From: Ali Firas @ 2026-09-09 9:26 UTC (permalink / raw) To: netdev, idosch Cc: kuba, pabeni, davem, edumazet, andrew+netdev, razor, roopa, linux-kernel, Ali Firas VXLAN_VNIFILTER_ENTRY_START and VXLAN_VNIFILTER_ENTRY_END are parsed without any bound on how far apart they are, so a single RTM_NEWTUNNEL message can ask for the whole 24-bit VNI space. vxlan_vni_add_del() then loops over that span creating one VNI node and one per-CPU stats block per iteration, all under rtnl_lock. Two things follow from that, both reachable by an unprivileged user in a user+network namespace, since adding VNIs only requires CAP_NET_ADMIN in the network namespace's user namespace: - the allocation is unbounded. Each VNI costs 128 bytes of slab plus 64 bytes per possible CPU, so a full in-range request costs roughly 2.1 GiB + 1 GiB per possible CPU. Measured in a QEMU guest on 2, 4 and 8 CPU configurations, every one of them ends in a global OOM with the allocating task in vxlan_vnifilter_process(). No errno is returned because the calling process is itself OOM-killed, and the OOM killer also killed unrelated root-owned processes. - rtnl_lock is held for the entire loop. rtnl is global rather than per-netns, so unrelated network configuration blocks everywhere for as long as the request runs. Measured with a plain "ip link add dummy0 type dummy" in a different network namespace: it takes 0.011 s normally, 4.472 s while a 1,000,000 VNI request runs, and during a full-range request it never completes at all. Cap the span of one request at 4096 VNIs. The limit is on a single request, not on how many VNIs a device may hold: a device can still be populated with the whole VNI space, it just takes more than one message. The value follows from how the interface is used in practice, on bridged VXLAN devices where the VNI is derived from the VLAN and so cannot exceed the 4094 usable VLAN IDs. The limit is written as a driver-local constant rather than reusing VLAN_N_VID. The two numbers coincide today, but a bound on a VXLAN netlink request is not a count of VLAN IDs, and tying them together would make a change to one silently change the other. The check sits in vxlan_process_vni_filter(), where the span is known and before any VNI is created, so it rejects the request before any work is done. Both RTM_NEWTUNNEL and RTM_DELTUNNEL reach vxlan_vni_add_del() through this one function, so a single check covers add and delete. The span is inclusive, so START=0 END=4095 is 4096 VNIs and is accepted, while START=0 END=4096 is 4097 and is rejected. A request carrying only END has START default to 0 and is bounded the same way. A start above the end selects no VNI at all and is deliberately left behaving as it does today, rather than being turned into an error by unsigned wraparound. Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device") Assisted-by: LLM Signed-off-by: Ali Firas <alishmery18@gmail.com> --- drivers/net/vxlan/vxlan_vnifilter.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c index dd94085e0886..f18ce0e1e741 100644 --- a/drivers/net/vxlan/vxlan_vnifilter.c +++ b/drivers/net/vxlan/vxlan_vnifilter.c @@ -17,6 +17,14 @@ #include "vxlan_private.h" +/* Maximum number of VNIs a single RTM_NEWTUNNEL or RTM_DELTUNNEL request may + * span. VNI filtering is mainly used on bridged VXLAN devices where the VNI + * is derived from the VLAN, so a span wider than the VLAN ID space has no + * practical use, while an unbounded span lets one netlink message create up + * to 2^24 VNIs under rtnl_lock. + */ +#define VXLAN_VNI_FILTER_RANGE_MAX 4096 + static inline int vxlan_vni_cmp(struct rhashtable_compare_arg *arg, const void *ptr) { @@ -869,6 +877,17 @@ static int vxlan_process_vni_filter(struct vxlan_dev *vxlan, return -EINVAL; } + /* Only bound a well-formed range; a start above the end selects no + * VNI at all and is left behaving as before. + */ + if (vni_end >= vni_start && + vni_end - vni_start >= VXLAN_VNI_FILTER_RANGE_MAX) { + NL_SET_ERR_MSG_ATTR_FMT(extack, nlvnifilter, + "VNI range spans more than %u VNIs", + VXLAN_VNI_FILTER_RANGE_MAX); + return -EINVAL; + } + if (vattrs[VXLAN_VNIFILTER_ENTRY_GROUP]) { group.sin.sin_addr.s_addr = nla_get_in_addr(vattrs[VXLAN_VNIFILTER_ENTRY_GROUP]); -- 2.53.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request 2026-09-09 9:26 ` [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request Ali Firas @ 2026-09-10 9:38 ` netdev-bot+sashiko 0 siblings, 0 replies; 13+ messages in thread From: netdev-bot+sashiko @ 2026-09-10 9:38 UTC (permalink / raw) To: alishmery18 Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev, razor, roopa, linux-kernel Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: Critical: 0 · High: 2 · Medium: 1 · Low: 0 - [High] The new cap is enforced per VXLAN_VNIFILTER_ENTRY attribute, not per netlink request, so the bug class the commit claims to close stays… - [High] The added check can be bypassed entirely because vxlan_vni_add_del() (drivers/net/vxlan/vxlan_vnifilter.c:820) iterates… - [Medium] The kernel now emits VNI-filter messages it will refuse to parse. -- vxlan: vnifilter: limit the VNI range of a single request This adds a driver-local constant VXLAN_VNI_FILTER_RANGE_MAX of 4096 and rejects a VXLAN_VNIFILTER_ENTRY_START/END pair whose span is wider than that, in vxlan_process_vni_filter(). The stated goal is to stop one RTM_NEWTUNNEL or RTM_DELTUNNEL message from creating up to 2^24 VNIs under rtnl_lock. > diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c > index dd94085e0886..f18ce0e1e741 100644 > --- a/drivers/net/vxlan/vxlan_vnifilter.c > +++ b/drivers/net/vxlan/vxlan_vnifilter.c > @@ -17,6 +17,14 @@ > > #include "vxlan_private.h" > > +/* Maximum number of VNIs a single RTM_NEWTUNNEL or RTM_DELTUNNEL request may > + * span. VNI filtering is mainly used on bridged VXLAN devices where the VNI > + * is derived from the VLAN, so a span wider than the VLAN ID space has no > + * practical use, while an unbounded span lets one netlink message create up > + * to 2^24 VNIs under rtnl_lock. > + */ > +#define VXLAN_VNI_FILTER_RANGE_MAX 4096 > + > static inline int vxlan_vni_cmp(struct rhashtable_compare_arg *arg, > const void *ptr) > { [ ... ] > @@ -869,6 +877,17 @@ static int vxlan_process_vni_filter(struct vxlan_dev *vxlan, > return -EINVAL; > } > > + /* Only bound a well-formed range; a start above the end selects no > + * VNI at all and is left behaving as before. > + */ > + if (vni_end >= vni_start && > + vni_end - vni_start >= VXLAN_VNI_FILTER_RANGE_MAX) { > + NL_SET_ERR_MSG_ATTR_FMT(extack, nlvnifilter, > + "VNI range spans more than %u VNIs", > + VXLAN_VNI_FILTER_RANGE_MAX); > + return -EINVAL; > + } > + [Severity: High] Is this bound per request, or per VXLAN_VNIFILTER_ENTRY attribute? vxlan_process_vni_filter() is called once per entry from vxlan_vnifilter_process(): nlmsg_for_each_attr_type(attr, VXLAN_VNIFILTER_ENTRY, nlh, sizeof(*tmsg), rem) { err = vxlan_process_vni_filter(vxlan, attr, nlh->nlmsg_type, extack); vnis++; if (err) break; } The vnis counter is only used afterwards to emit "No vnis found to process", it is never a budget, and vni_filter_policy places no limit on how many nested VXLAN_VNIFILTER_ENTRY attributes a message may carry: static const struct nla_policy vni_filter_policy[VXLAN_VNIFILTER_MAX + 1] = { [VXLAN_VNIFILTER_ENTRY] = { .type = NLA_NESTED }, }; A minimal entry is a 4 byte nest header plus an 8 byte START and an 8 byte END, so a message that fits inside the default sk_sndbuf can carry on the order of ten thousand entries, each of which passes the new check while spanning 4096 VNIs. That sums to more than the entire 24-bit space in one message. The handlers are registered with flags 0: {THIS_MODULE, PF_BRIDGE, RTM_NEWTUNNEL, vxlan_vnifilter_process, NULL, 0}, {THIS_MODULE, PF_BRIDGE, RTM_DELTUNNEL, vxlan_vnifilter_process, NULL, 0}, so rtnl_lock is held for the whole loop, with no lock drop and no cond_resched(), and each iteration of vxlan_vni_add_del() still does kzalloc_obj() plus netdev_alloc_pcpu_stats() per VNI in vxlan_vni_alloc(). Would the bound need to accumulate the span across all entries of the message, or be applied in vxlan_vnifilter_process() before any entry is handed to vxlan_process_vni_filter()? The commit message says: "The check sits in vxlan_process_vni_filter(), where the span is known and before any VNI is created, so it rejects the request before any work is done." Since entries are parsed and applied one at a time, an oversized entry in the middle of a message returns -EINVAL only after all preceding entries have already created their VNI nodes and sent their notifications. Should that sentence be adjusted, along with the comment above the constant which describes the value as a cap on what "a single RTM_NEWTUNNEL or RTM_DELTUNNEL request may span"? [Severity: High] Can this check be skipped entirely with a large START value? vni_filter_entry_policy declares the two attributes without any maximum: [VXLAN_VNIFILTER_ENTRY_START] = { .type = NLA_U32 }, [VXLAN_VNIFILTER_ENTRY_END] = { .type = NLA_U32 }, so START = 0xffffffff and no END gives vni_start == vni_end == 0xffffffff. The "!vni_start && !vni_end" test above does not fire, and here vni_end - vni_start is 0, so this looks like a one VNI request and is accepted. vxlan_vni_add_del() then iterates with a signed counter: int v, err = 0; ... for (v = start_vni; v <= end_vni; v++) { v starts at -1, and in "v <= end_vni" it is converted to unsigned against the __u32 end_vni, so 0xffffffff <= 0xffffffff is true, a VNI is created, v++ makes v == 0, and 0 <= 0xffffffff is still true. Does the loop then keep walking upwards from 0, creating a node plus per-CPU stats plus a notification per iteration under rtnl_lock, until -ENOMEM or until rhashtable returns -E2BIG (vxlan_vni_rht_params has .max_size = VXLAN_N_VID, so max_elems is about 33.5M)? vxlan_vni_in_use() returns 0 for a namespace holding a single vnifilter device, so nothing seems to abort it earlier. Once v passes INT_MAX the increment is also signed overflow. The same applies to any range that ends at 0xffffffff, for example START = 0xfffff001 with END = 0xffffffff, whose span is 4095. Would bounding the absolute values (or making the loop counter unsigned) be needed in addition to bounding the difference? [Severity: Medium] Does this make the dump output unparseable by the same code path? vxlan_vnifilter_dump_dev() merges an unbounded run of contiguous VNIs that share a remote into one VXLAN_VNIFILTER_ENTRY: if (!dump_stats && vnirange(vend, v) == 1 && vxlan_addr_equal(&v->remote_ip, &vend->remote_ip)) { goto update_end; and then calls vxlan_fill_vni_filter_entry(skb, vbegin, vend, dump_stats) once for the merged run, emitting a single START/END pair with no clamp on the span. The commit message keeps it legal for a device to hold the whole VNI space through several requests, so a device populated with, say, 100000 contiguous VNIs via 25 accepted requests is dumped as one entry with END - START == 99999. Replaying that entry, or deleting the run with one RTM_DELTUNNEL, now returns -EINVAL with "VNI range spans more than 4096 VNIs". Should the dump side chunk merged runs to the same maximum span, or should the input restriction be expressed as something other than a limit on the START/END span? -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909092645.3105263-1-alishmery18%40gmail.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg 2026-09-09 9:26 ` [PATCH net 0/3] vxlan: vnifilter: bound the VNI range per request Ali Firas 2026-09-09 9:26 ` [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request Ali Firas @ 2026-09-09 9:26 ` Ali Firas 2026-09-10 9:38 ` netdev-bot+sashiko 2026-09-09 9:26 ` [PATCH net 3/3] selftests: net: test the vxlan vnifilter VNI range limit Ali Firas 2 siblings, 1 reply; 13+ messages in thread From: Ali Firas @ 2026-09-09 9:26 UTC (permalink / raw) To: netdev, idosch Cc: kuba, pabeni, davem, edumazet, andrew+netdev, razor, roopa, linux-kernel, Ali Firas vxlan_vni_alloc() allocates a struct vxlan_vni_node and a per-CPU stats block for every VNI, both with plain GFP_KERNEL. Neither carries __GFP_ACCOUNT, so the memory is not charged to the cgroup of the process that asked for it. With the range of a single request now capped, one message can no longer exhaust memory on its own. This is no longer the primary defence, but it still matters: nothing limits how many capped requests a task may issue, so an unprivileged user in a user+network namespace can still accumulate an arbitrary number of VNIs, 4096 at a time, and none of it is charged to them. Per VNI the add path allocates 128 bytes of slab, an exact fit in kmalloc-128 and measured at exactly 1.000 objects per VNI, plus 64 bytes per possible CPU for the stats block. The per-CPU term is the one that grows: 256 bytes per VNI on a 2-CPU host, but 4.2 KB per VNI on a 64-CPU one. Charging both allocations confines the damage to the caller's cgroup. The kill becomes CONSTRAINT_MEMCG with oom_memcg set to that cgroup, memory.stat attributes both the slab and the percpu bytes to it, and the host survives what previously took it down. One limitation is worth stating plainly: try_charge() reclaims and then invokes the memcg OOM killer rather than returning -ENOMEM, so the request does not fail gracefully, the caller is killed. Accounting confines the blast radius, it does not turn this into a clean error. For a caller not under a memcg limit there is no change. With no limit set, the same workload installs the same number of VNIs to within 0.4%, fails at the same point, and a bounded add of 1,000,000 VNIs costs an identical 128 bytes of slab and 64 bytes per CPU. The objects simply move from kmalloc-128 to kmalloc-cg-128. Conditions to recreate the bug: - CONFIG_VXLAN, CONFIG_MEMCG. - Unprivileged user in a fresh user+network namespace (unshare -Urn), or root with CAP_NET_ADMIN. - Create a vnifilter-enabled vxlan device and add VNIs in a loop (e.g. ip link add vx0 type vxlan external vnifilter dstport 4789, then repeated bridge vni add ... commands) while watching a memcg-limited cgroup: system slab and percpu grow far faster than memory.current, pinning kernel memory outside memcg charging. Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device") Assisted-by: LLM Signed-off-by: Ali Firas <alishmery18@gmail.com> --- drivers/net/vxlan/vxlan_vnifilter.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c index f18ce0e1e741..3d6718ec3f55 100644 --- a/drivers/net/vxlan/vxlan_vnifilter.c +++ b/drivers/net/vxlan/vxlan_vnifilter.c @@ -703,10 +703,11 @@ static struct vxlan_vni_node *vxlan_vni_alloc(struct vxlan_dev *vxlan, { struct vxlan_vni_node *vninode; - vninode = kzalloc_obj(*vninode); + vninode = kzalloc_obj(*vninode, GFP_KERNEL_ACCOUNT); if (!vninode) return NULL; - vninode->stats = netdev_alloc_pcpu_stats(struct vxlan_vni_stats_pcpu); + vninode->stats = __netdev_alloc_pcpu_stats(struct vxlan_vni_stats_pcpu, + GFP_KERNEL_ACCOUNT); if (!vninode->stats) { kfree(vninode); return NULL; -- 2.53.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg 2026-09-09 9:26 ` [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg Ali Firas @ 2026-09-10 9:38 ` netdev-bot+sashiko 0 siblings, 0 replies; 13+ messages in thread From: netdev-bot+sashiko @ 2026-09-10 9:38 UTC (permalink / raw) To: alishmery18 Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev, razor, roopa, linux-kernel Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 0 · Low: 2 - [Low] The commit message's quantitative model of the per-CPU charge is wrong/incomplete. - [Low] The commit message states categorically that 'try_charge() reclaims and then invokes the memcg OOM killer rather than returning… -- vxlan: vnifilter: account VNI node and per-CPU stats to memcg This changes vxlan_vni_alloc() to allocate the struct vxlan_vni_node with GFP_KERNEL_ACCOUNT and to switch the per-VNI stats block from netdev_alloc_pcpu_stats() to __netdev_alloc_pcpu_stats() with GFP_KERNEL_ACCOUNT, so both allocations are charged to the caller's memcg. Both comments below are about the numbers and the failure behaviour described in the changelog, not about the code change itself. > diff --git a/drivers/net/vxlan/vxlan_vnifilter.c b/drivers/net/vxlan/vxlan_vnifilter.c > index f18ce0e1e741..3d6718ec3f55 100644 > --- a/drivers/net/vxlan/vxlan_vnifilter.c > +++ b/drivers/net/vxlan/vxlan_vnifilter.c > @@ -703,10 +703,11 @@ static struct vxlan_vni_node *vxlan_vni_alloc(struct vxlan_dev *vxlan, > { > struct vxlan_vni_node *vninode; > > - vninode = kzalloc_obj(*vninode); > + vninode = kzalloc_obj(*vninode, GFP_KERNEL_ACCOUNT); > if (!vninode) > return NULL; > - vninode->stats = netdev_alloc_pcpu_stats(struct vxlan_vni_stats_pcpu); > + vninode->stats = __netdev_alloc_pcpu_stats(struct vxlan_vni_stats_pcpu, > + GFP_KERNEL_ACCOUNT); > if (!vninode->stats) { > kfree(vninode); > return NULL; [Severity: Low] Is the stated cost model for this per-CPU allocation complete? The changelog says: "plus 64 bytes per possible CPU for the stats block. The per-CPU term is the one that grows: 256 bytes per VNI on a 2-CPU host, but 4.2 KB per VNI on a 64-CPU one." Once __GFP_ACCOUNT is set, the amount charged comes from pcpu_obj_full_size() in mm/percpu-internal.h: size_t extra_size = 0; #ifdef CONFIG_MEMCG if (!mem_cgroup_kmem_disabled()) extra_size += size / PCPU_MIN_ALLOC_SIZE * sizeof(struct obj_cgroup *); #endif return size * num_possible_cpus() + extra_size; For a 64-byte object on a 64-bit build that extra term is 64 / 4 * 8 = 128 bytes, which is what makes the quoted measurements add up (64 * 2 + 128 = 256, 64 * 64 + 128 = 4224). Would it be clearer to state the obj_cgroup metadata term alongside the per-CPU term, since the stated formula and the stated measurements otherwise disagree? Also, is the 64-byte figure architecture independent? struct vxlan_vni_stats_pcpu in include/net/vxlan.h embeds a u64_stats_sync: struct vxlan_vni_stats_pcpu { struct vxlan_vni_stats stats; struct u64_stats_sync syncp; }; That member is empty on 64-bit but carries a seqcount on 32-bit, so the object is larger than 64 bytes there. [Severity: Low] The changelog states: "try_charge() reclaims and then invokes the memcg OOM killer rather than returning -ENOMEM, so the request does not fail gracefully, the caller is killed" Is that categorical? In try_charge_memcg() in mm/memcontrol.c, when mem_cgroup_oom() returns false (no killable task, oom_lock bypass, an existing OOM victim with MMF_OOM_SKIP, or task_in_memcg_oom(current)), control reaches: nomem: if (!(gfp_mask & (__GFP_NOFAIL | __GFP_HIGH))) return -ENOMEM; GFP_KERNEL_ACCOUNT carries neither __GFP_NOFAIL nor __GFP_HIGH, so the charge can return -ENOMEM, vxlan_vni_alloc() then returns NULL and the netlink request does fail with -ENOMEM rather than the caller being killed. Separately, when the memcg OOM killer does run it picks a victim inside the constrained cgroup, which need not be the allocating task, and memory.oom.group can extend the kill to the whole cgroup. Would it be more accurate to say the add may trigger memcg reclaim and then either a memcg OOM kill of one or more tasks in the cgroup, or a plain -ENOMEM? -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909092645.3105263-1-alishmery18%40gmail.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net 3/3] selftests: net: test the vxlan vnifilter VNI range limit 2026-09-09 9:26 ` [PATCH net 0/3] vxlan: vnifilter: bound the VNI range per request Ali Firas 2026-09-09 9:26 ` [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request Ali Firas 2026-09-09 9:26 ` [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg Ali Firas @ 2026-09-09 9:26 ` Ali Firas 2026-09-10 9:39 ` netdev-bot+sashiko 2 siblings, 1 reply; 13+ messages in thread From: Ali Firas @ 2026-09-09 9:26 UTC (permalink / raw) To: netdev, idosch Cc: kuba, pabeni, davem, edumazet, andrew+netdev, razor, roopa, linux-kernel, Ali Firas A single RTM_NEWTUNNEL or RTM_DELTUNNEL request may span at most 4096 VNIs. Nothing covered the range functionality at all, so add the boundary cases to the existing API test: a range of exactly the maximum is accepted and one VNI more is rejected, for both add and delete. The range used sits above the VNIs the surrounding tests already install, so it does not disturb them. Assisted-by: LLM Signed-off-by: Ali Firas <alishmery18@gmail.com> --- .../selftests/net/test_vxlan_vnifiltering.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh index 8deacc565afa..464ff353d6c0 100755 --- a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh +++ b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh @@ -371,6 +371,19 @@ vxlan_vnifilter_api() # change vxlan vnifilter flag run_cmd "ip -netns $testns link set dev vxlan-ext1 type vxlan external novnifilter" log_test $? 2 "Cannot unset vnifilter flag on a device" + + # a single request may span at most 4096 vnis + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 10000-14095" + log_test $? 0 "Add vni range of maximum size" + + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 10000-14096" + log_test $? 255 "Cannot add vni range larger than maximum" + + run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 10000-14096" + log_test $? 255 "Cannot delete vni range larger than maximum" + + run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 10000-14095" + log_test $? 0 "Delete vni range of maximum size" } # Sanity test vnifilter datapath -- 2.53.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net 3/3] selftests: net: test the vxlan vnifilter VNI range limit 2026-09-09 9:26 ` [PATCH net 3/3] selftests: net: test the vxlan vnifilter VNI range limit Ali Firas @ 2026-09-10 9:39 ` netdev-bot+sashiko 0 siblings, 0 replies; 13+ messages in thread From: netdev-bot+sashiko @ 2026-09-10 9:39 UTC (permalink / raw) To: alishmery18 Cc: netdev, idosch, kuba, pabeni, davem, edumazet, andrew+netdev, razor, roopa, linux-kernel Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Critical: 0 · High: 1 · Medium: 1 · Low: 0 - [High] The series' stated guarantee — "A single RTM_NEWTUNNEL or RTM_DELTUNNEL request may span at most 4096 VNIs" (this selftest's… - [Medium] The new "Cannot delete vni range larger than maximum" case (tools/testing/selftests/net/test_vxlan_vnifiltering.sh:382-383) has… -- selftests: net: test the vxlan vnifilter VNI range limit This adds four cases to vxlan_vnifilter_api() in test_vxlan_vnifiltering.sh covering the new 4096-VNI cap on a vnifilter request: an add and a delete of exactly the maximum span are expected to succeed, and an add and a delete one VNI larger are expected to fail. No Link: tags were present in the commit header. > diff --git a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh > index 8deacc565afa..464ff353d6c0 100755 > --- a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh > +++ b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh > @@ -371,6 +371,19 @@ vxlan_vnifilter_api() > # change vxlan vnifilter flag > run_cmd "ip -netns $testns link set dev vxlan-ext1 type vxlan external novnifilter" > log_test $? 2 "Cannot unset vnifilter flag on a device" > + > + # a single request may span at most 4096 vnis > + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 10000-14095" > + log_test $? 0 "Add vni range of maximum size" > + > + run_cmd "bridge -netns $testns vni add dev vxlan-ext1 vni 10000-14096" > + log_test $? 255 "Cannot add vni range larger than maximum" [Severity: High] The comment here, and the commit message wording "A single RTM_NEWTUNNEL or RTM_DELTUNNEL request may span at most 4096 VNIs", describe a per-request property. Is that what the companion patch actually enforces? The check lives in vxlan_process_vni_filter(): if (vni_end >= vni_start && vni_end - vni_start >= VXLAN_VNI_FILTER_RANGE_MAX) { NL_SET_ERR_MSG_ATTR_FMT(extack, nlvnifilter, "VNI range spans more than %u VNIs", VXLAN_VNI_FILTER_RANGE_MAX); return -EINVAL; } and vxlan_vnifilter_process() calls that helper once per VXLAN_VNIFILTER_ENTRY attribute present in the message: nlmsg_for_each_attr_type(attr, VXLAN_VNIFILTER_ENTRY, nlh, sizeof(*tmsg), rem) { err = vxlan_process_vni_filter(vxlan, attr, nlh->nlmsg_type, extack); vnis++; if (err) break; } So a message carrying N entries, each an individually legal 4096-VNI span, still results in N * 4096 add/del operations inside one rtnl_lock hold. An entry nest with only START and END is roughly 20 bytes on the wire, and netlink_sendmsg() accepts messages up to sk_sndbuf/2, so thousands of entries fit in a single request. Since iproute2 emits exactly one VXLAN_VNIFILTER_ENTRY per message, can any of the four new cases observe a multi-entry request? They all exercise the per-entry path only. Would it make sense either to accumulate the span across entries in vxlan_vnifilter_process(), or to reword the comment above VXLAN_VNI_FILTER_RANGE_MAX, the commit messages and these test names to say "per entry"? > + > + run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 10000-14096" > + log_test $? 255 "Cannot delete vni range larger than maximum" > + > + run_cmd "bridge -netns $testns vni del dev vxlan-ext1 vni 10000-14095" > + log_test $? 0 "Delete vni range of maximum size" [Severity: Medium] Can the "Cannot delete vni range larger than maximum" case distinguish the range limit from a plain missing-VNI error? Only 10000-14095 were installed by the first case, so the delete of 10000-14096 has two ways to return non-zero. With the cap present, vxlan_process_vni_filter() returns -EINVAL and nothing is touched. With the cap removed, vxlan_vni_add_del() deletes 10000-14095 and then vxlan_vni_del() hits: vninode = rhashtable_lookup_fast(&vg->vni_hash, &v, vxlan_vni_rht_params); if (!vninode) { err = -ENOENT; goto out; } which the loop propagates. iproute2 maps either errno to exit status 255, the same coarse value the pre-existing "vni add dev vxlan-ext2 vni 200" case already expects, so log_test $? 255 passes in both cases. In the no-cap case the 4096 VNIs are already gone, so the next assertion, "Delete vni range of maximum size" expecting 0, is the one that fails. Does that not report the regression under the wrong test name? Would deleting a span whose every VNI exists (for example installing 10000-14096 with two capped adds first), or matching the extack text rather than only the exit status, make the case specific to the limit? -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909092645.3105263-1-alishmery18%40gmail.com ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-09-10 9:39 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-02 15:46 [PATCH net v2] vxlan: vnifilter: validate the VNI range in vni_filter_entry_policy Ali Firas 2026-09-05 19:36 ` netdev-bot+sashiko 2026-09-05 19:39 ` Jakub Kicinski 2026-09-06 23:16 ` Ali Firas 2026-09-07 10:52 ` Ali Firas 2026-09-07 14:10 ` Ido Schimmel 2026-09-09 9:26 ` [PATCH net 0/3] vxlan: vnifilter: bound the VNI range per request Ali Firas 2026-09-09 9:26 ` [PATCH net 1/3] vxlan: vnifilter: limit the VNI range of a single request Ali Firas 2026-09-10 9:38 ` netdev-bot+sashiko 2026-09-09 9:26 ` [PATCH net 2/3] vxlan: vnifilter: account VNI node and per-CPU stats to memcg Ali Firas 2026-09-10 9:38 ` netdev-bot+sashiko 2026-09-09 9:26 ` [PATCH net 3/3] selftests: net: test the vxlan vnifilter VNI range limit Ali Firas 2026-09-10 9:39 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox