Re: [PATCH nf] netfilter: ipset: restore element before rescanning cidrs on seqcount retry

[Florian Westphal]

Tamaki Yanagawa <ty@xxxxxxxxx> wrote:
> mtype_test_cidrs() masks the element under test in place with
> mtype_data_netmask() while walking the cidr array from the most to the
> least specific prefix. Since the walk is restarted on concurrent cidr
> array updates, the IPSET_NET_COUNT == 1 types (hash:net, hash:net,port,
> hash:ip,port,net, hash:net,iface) rescan with an element already masked
> to the widest prefix tried, so a lookup can wrongly match a more specific
> entry at the network base address (e.g. 10.200.3.4 matching 10.0.0.0/29).
> Restore the element before rescanning.

Is this a real world encountered bug or an LLM finding?

> +	/* mtype_data_netmask() masks @d in place, undo it before rescanning */
> +	*d = orig;
> +	goto retry;

Why do we need to do this instead of not masking @d in place to begin
with?

-mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
+mtype_test_cidrs(struct ip_set *set, const struct mtype_elem *orig,

+       struct mtype_elem *data, d;
        struct hbucket *n;
-       struct mtype_elem *data;

 retry:
+       d = *orig;


... and fix up all reset calls:

 static void
 hash_netportnet6_data_reset_elem(struct hash_netportnet6_elem *elem,
-                                struct hash_netportnet6_elem *orig)
+                                const struct hash_netportnet6_elem *orig)

... much cleaner and less error prone, I think.