Re: [PATCH net-next v4 0/7] vxlan: vnifilter: bound a single request and account per-VNI memory

Re: [PATCH net-next v4 0/7] vxlan: vnifilter: bound a single request and account per-VNI memory

[Ali Firas]

On 10/14/26 15:45, netdev-bot+sinfo@kernel.org wrote:
> - How the issue was discovered

Manual source review of the vnifilter netlink path, after noticing that
vxlan_mdb.c range-validates its VNI attributes while vxlan_vnifilter.c
does not. Prepared with LLM assistance, as the Assisted-by: tags say.

> - Whether the issue was actually triggered, or is only theoretical

Triggered and measured in QEMU, from a user namespace holding
CAP_NET_ADMIN only inside it. There is no stack trace to quote: the
symptoms are rtnl hold time and uncharged memory growth, not a crash.
The changelogs carry the numbers.

The cover letter states this is a policy and hardening change rather
than a fix, targeting net-next with no Fixes tag, at Jakub's request:

  https://lore.kernel.org/netdev/20260921150904.65a704eb@kernel.org/

Re: [PATCH net-next v4 0/7] vxlan: vnifilter: bound a single request and account per-VNI memory

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

[PATCH net-next v4 0/7] vxlan: vnifilter: bound a single request and account per-VNI memory

[Ali Firas]

The VNI filter interface accepts a START/END range with no bound on
either endpoint and no bound on how many VNIs one message may ask for,
and the memory it allocates per VNI is not charged to the caller's
cgroup.

Patch 1 links the VXLAN_VNIFILTER_ENTRY nest to its policy with
NLA_POLICY_NESTED(), as Jakub asked in v2:

  https://lore.kernel.org/netdev/20260921150904.65a704eb@kernel.org/

Without it the entry attributes were validated only as each entry was
dispatched, so a multi-entry message whose later entry was invalid had
the earlier entries applied and notified before the message was
rejected. With the nest linked, an entry the policy rejects refuses the
message as a unit; faults the policy does not cover are still handled
one entry at a time, as before.

Patch 2 range-validates both endpoints to the 24-bit VNI space.

Patch 3 makes the dump re-enterable, and does so before patch 4 caps the
input, so no commit in the series reports a range its own input path
would reject. vxlan_vnifilter_dump_dev() coalesces a contiguous run of
VNIs, and packs as many entries into a netlink message as the skb holds,
with no bound on either. It introduces VXLAN_VNI_FILTER_MSG_MAX and
bounds both: it ends a coalesced run at the limit, and carries a
per-message VNI budget that ends the message once its entries would sum
past the limit, the same way the loop already ends it when the skb is
full. The same resume path (cb->args[1], counting nodes dumped) drives
both; the dump does not drop or repeat a VNI.

Patch 4 caps the number of VNIs one request may add or delete, summed
over its entries, at VXLAN_VNI_FILTER_MSG_MAX. Both add and delete walk
the span one VNI at a time under rtnl_lock, so both are bounded; that
walk is the cost, not the memory.

Patch 5 charges the per-VNI node and its per-CPU stats block to the
cgroup of the task that created the VNI, the way the device's own struct
and its Tx/Rx queues already are (commit c948f51c1654 ("memcg: enable
accounting for net_device and Tx/Rx queues")).

Patch 6 charges the default FDB entry a VNI with a group or remote
installs -- the vxlan_fdb, its vxlan_rdst and the per-CPU dst cache. It
threads a gfp argument through vxlan_fdb_update() down to
vxlan_fdb_alloc()/vxlan_fdb_append(), on both the create and the
remote-change paths, and a second gfp through
vxlan_update_default_fdb_entry() so each caller picks it. A per-request
add or change (RTM_NEWTUNNEL) passes GFP_NOWAIT | __GFP_ACCOUNT, which
is non-blocking like GFP_ATOMIC but hard-bounded by the memcg: at its
limit the request fails with -ENOMEM instead of charging past
memory.max, and because the new entry is allocated before the old is
freed, a failed add leaves the existing entry intact. The device-wide
default-remote change (RTM_NEWLINK, via vxlan_vnilist_update_group())
keeps GFP_ATOMIC | __GFP_ACCOUNT; that pre-existing walk is not bounded
here ("Not addressed" below). Every non-vnifilter caller --
vxlan_fdb_add(), vxlan_snoop(), vxlan_dev_create(), vxlan_changelink()
and the switchdev vxlan_fdb_external_learn_add() -- keeps plain
GFP_ATOMIC, so the data path and unrelated callers are untouched.

Patch 7 adds the selftests.

This is the reasoning of commit e2c8d550a973 ("netfilter: ebtables:
account ebt_table_info to kmemcg") and of the net_device accounting
above: memory whose lifetime is tied to the net namespace rather than to
the allocating task cannot be relieved by the OOM killer, so a
container's netadmin must not be able to grow it outside the memcg that
bounds the container.

What this series does and does not account, measured on production
builds (no lockdep/KASAN) in a memcg at 2 and 8 vCPU:

  - every persistent per-VNI allocation the vnifilter path creates is
    attributed to the cgroup of the task whose request allocates it, and
    bounded by it on the per-request path. Patch 5's node (128 B) and
    per-CPU stats (64 B/CPU), and patch 6's FDB entry -- vxlan_fdb
    (128 B), vxlan_rdst (96 B) and a per-CPU dst cache -- on the
    per-request path (RTM_NEWTUNNEL) are hard-bounded: the node and stats
    use GFP_KERNEL_ACCOUNT and the FDB entry uses
    GFP_NOWAIT | __GFP_ACCOUNT, so a cgroup at its limit fails the
    create, the first remote, or a remote change with -ENOMEM, and
    memory.current does not exceed memory.max (measured at 2 and 8 vCPU
    in an 8 MiB cgroup). Failure is clean: the new entry is allocated
    before the old is freed, so a failed add leaves the existing entry
    intact and returns the error -- no established state is destroyed and
    no false success is returned. The at-rest footprint is one node, its
    stats and one FDB entry per VNI; this memory has device and net
    namespace lifetime, not task lifetime (killing the cgroup's tasks did
    not release it, only deleting the device did), so it must be charged
    rather than left to the OOM killer;
  - the rhashtable bucket tables backing the VNI and FDB hashes are not
    charged. That is ~32 bytes per VNI across both tables, generic
    infrastructure shared by every rhashtable user, O(entries) with
    automatic shrinking (an add/delete cycle returns it, measured), and
    the series does not address it.

The cap is symmetric, applied to add and delete alike, which is what Ido
asked for when he agreed to a 4k limit. Patch 3 is what makes that safe:
no dumped message carries more VNIs than one request may ask for, so a
device holding more than the limit can still be torn down by replaying
what "bridge vni show" reports. Device teardown itself
(vxlan_vnigroup_uninit()) is not a netlink message and is not subject to
the cap.

uAPI changes, all of them:

  - a VNI at or above 2^24, previously accepted and stored (and
    truncated on the wire), is now rejected with -ERANGE;
  - a single request asking to add or delete more than 4096 VNIs, summed
    over its entries, is now rejected with -EINVAL -- "bridge vni add
    dev X vni 1-10000" used to be accepted;
  - a request rejected by the entry policy, aimed at a missing or
    non-vnifilter device, now returns that policy error rather than
    -ENODEV or -EOPNOTSUPP, because the nest is validated before the
    device is resolved (patch 1);
  - "bridge vni show" now splits a contiguous run longer than the limit
    into several entries, and across several messages, instead of one;
    the set of VNIs it reports is unchanged;
  - in a cgroup at its memory limit, adding a VNI, or adding or changing
    a VNI's remote (RTM_NEWTUNNEL), now returns -ENOMEM instead of
    charging past the limit; the existing entry, if any, is left intact.

This is a policy and hardening change, not a fix for a crash or
corruption; it targets net-next and should not be backported.

One pre-existing semantic is worth review: an entry that carries only
END and no START is treated as the range [0, END], so a lone END near
the top of the space is a whole-space request that the cap now rejects;
whether END-without-START should mean that is left as an open question.

Not addressed here:

  - the device-wide default-remote change (RTM_NEWLINK) reaches the same
    per-VNI FDB entries outside the request cap and force-charges them
    rather than hard-bounding, so this series narrows the exposure to
    the per-request route rather than closing it. Patch 6 does charge
    those entries to the requesting task's cgroup, where before the
    series they grew unaccounted host memory; bounding the walk is left
    to a follow-up.
  - vxlan_vni_add_del() still leaves the earlier VNIs of a range
    installed if an allocation fails partway through it; with the cap
    that is now bounded to fewer than 4096 VNIs. A fix needs a
    transaction and is left to a follow-up.
  - the rhashtable bucket memory described above.

Changes since v3:
  - rebased onto current net-next.
  - the dump patch (now patch 3) is ordered before the input cap (patch
    4) and introduces VXLAN_VNI_FILTER_MSG_MAX, so every commit is
    self-consistent; it also carries a per-message VNI budget, not only a
    per-entry clamp, so a dump of a run longer than the limit splits into
    messages the input path accepts (demonstrated with raw netlink).
  - new patch 6 accounts the default FDB entry, scoped to the vnifilter
    call site via a gfp argument so the data path stays on GFP_ATOMIC;
    the gfp is threaded on both the create and the remote-change paths.
    The per-request path (RTM_NEWTUNNEL) uses GFP_NOWAIT | __GFP_ACCOUNT
    so the charge is hard-bounded (fails with -ENOMEM at the cgroup
    limit); the pre-existing device-default walk (RTM_NEWLINK) keeps
    GFP_ATOMIC | __GFP_ACCOUNT.
  - patch 5 reworded to cover the node and stats only, with its
    net_device-accounting reference narrowed to net_device and its Tx/Rx
    queues.
  - patch 1 reworded to scope the "rejected as a unit" claim to entries
    the policy rejects.

Changes since v2:
  - add patch 1 linking the nest to its policy (Jakub);
  - the request cap applies to both add and delete (Ido).

Ali Firas (7):
  vxlan: vnifilter: validate the VXLAN_VNIFILTER_ENTRY nest
  vxlan: vnifilter: reject VNIs outside the 24-bit space
  vxlan: vnifilter: clamp the dumped VNI range to the request limit
  vxlan: vnifilter: bound the number of VNIs one request may touch
  vxlan: vnifilter: account per-VNI memory to memcg
  vxlan: vnifilter: account the default FDB entry to memcg
  selftests: net: test the vxlan vnifilter request limit and dump replay

 drivers/net/vxlan/vxlan_core.c                |  47 +++---
 drivers/net/vxlan/vxlan_private.h             |   5 +-
 drivers/net/vxlan/vxlan_vnifilter.c           | 132 +++++++++++++++---
 .../selftests/net/test_vxlan_vnifiltering.sh  | 119 +++++++++++++++-
 4 files changed, 270 insertions(+), 46 deletions(-)


base-commit: 05b5019b01fb9443b09ae4ed34af1135a80a84c8
--
2.53.0