Re: [PATCH net-next v4 2/7] vxlan: vnifilter: reject VNIs outside the 24-bit space

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] This patch fixes long-standing defects in vxlan_vni_add_del() and
  vni_filter_entry_policy, but it has no Fixes: tag and is posted to…

--

vxlan: vnifilter: reject VNIs outside the 24-bit space

This patch adds NLA_POLICY_FULL_RANGE() checks to
VXLAN_VNIFILTER_ENTRY_START and VXLAN_VNIFILTER_ENTRY_END, which limits
both to VXLAN_N_VID - 1. It also changes the loop counter in
vxlan_vni_add_del() from int to u32, so a range that ends at 0xffffffff
can no longer wrap.

> How many VNIs a single in-range request may span is a
> separate question, bounded by the next patch; that limit is not a policy
> check and does run in the handler.

[ ... ]

> Assisted-by: LLM
> Signed-off-by: Ali Firas <alishmery18@gmail.com>
>
> ---
> v4: no functional change; rebased onto net-next.

[Severity: Low]
Should this patch have a Fixes: tag and go to net instead of net-next?

The int loop counter and the plain NLA_U32 policy entries seem to date
from commit f9c4bb0b245c ("vxlan: vni filtering support on collect
metadata device"), which was merged in v5.18:

    int v, err = 0;
    ...
    for (v = start_vni; v <= end_vni; v++) {

Since then, a request that carries only
VXLAN_VNIFILTER_ENTRY_START=0xffffffff passes this check in
vxlan_process_vni_filter():

    if (!vni_start && !vni_end) {

The loop in vxlan_vni_add_del() then visits 0xffffffff, 0, 1, and so on.
Each pass allocates a vxlan_vni_node and a per-CPU stats block under
rtnl_lock, and the loop only stops on an error such as -E2BIG from the
rhashtable or -ENOMEM. VNIs at or above VXLAN_N_VID are also stored,
while vxlan_vni_field() truncates them on the wire.

Anyone with CAP_NET_ADMIN over the netns can reach this, including root
in a user namespace.

Without a Fixes: tag, will the 24-bit aliasing fix and the signed
overflow fix reach stable kernels?

A backport would also need the preceding commit "vxlan: vnifilter:
validate the VXLAN_VNIFILTER_ENTRY nest". Without its NLA_POLICY_NESTED()
link, the new range policy is only applied per entry, by the
nla_parse_nested() call in vxlan_process_vni_filter().

The commit message also says the span is "bounded by the next patch".
The next patch in the series looks like "vxlan: vnifilter: clamp the
dumped VNI range to the request limit". The cap itself is in "vxlan:
vnifilter: bound the number of VNIs one request may touch", and that
patch says it is not a backport candidate.

Could the reference be corrected so backporters get accurate dependency
information?

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