netfilter: ipset: rework cidr bookkeeping

According to sashiko, the current bookkeeping of cidr values are unsafe
on weakly-ordered architectures. Replace the in-place updating with an
RCU based method: create the new bookeeping structure, update and replace
the old one with the new. Downside that we need to allocate memory when
deleting a cidr entry - in case of memory pressure fall back to leave holes
which possibility is taken into account at evaluation time.

Thanks to Pablo (Pablo Neira Ayuso <pablo@netfilter.org>) and Cyntia
(Cynthia <cynthia@kosmx.dev>) for helping me in debugging which resulted
the patch "netfilter: ipset: allocate the proper memory for the generic
hash structure" on which this very patch depends.

Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This commit is contained in:
Jozsef Kadlecsik
2026-07-30 20:38:49 +02:00
committed by Pablo Neira Ayuso
parent 3082597033
commit 8e5fd2a55e
7 changed files with 183 additions and 94 deletions

View File

@@ -99,9 +99,15 @@ struct htable {
#endif
/* Book-keeping of the prefixes added to the set */
struct net_prefix {
u8 cidr; /* the cidr value */
u32 count; /* number of elements of this cidr */
};
struct net_prefixes {
u32 nets[IPSET_NET_COUNT]; /* number of elements for this cidr */
u8 cidr[IPSET_NET_COUNT]; /* the cidr value */
struct rcu_head rcu;
u8 len;
struct net_prefix nets[] __counted_by(len);
};
/* Compute the hash table size */
@@ -127,11 +133,6 @@ htable_size(u8 hbits)
#else
#define __CIDR(cidr, i) (cidr)
#endif
/* cidr + 1 is stored in net_prefixes to support /0 */
#define NCIDR_PUT(cidr) ((cidr) + 1)
#define NCIDR_GET(cidr) ((cidr) - 1)
#ifdef IP_SET_HASH_WITH_NETS_PACKED
/* When cidr is packed with nomatch, cidr - 1 is stored in the data entry */
#define DCIDR_PUT(cidr) ((cidr) - 1)
@@ -141,21 +142,11 @@ htable_size(u8 hbits)
#define DCIDR_GET(cidr, i) __CIDR(cidr, i)
#endif
#define INIT_CIDR(cidr, host_mask) \
DCIDR_PUT(((cidr) ? NCIDR_GET(cidr) : host_mask))
#define INIT_CIDR(n, host_mask) ({ \
const struct net_prefixes *__n = rcu_dereference(n); \
DCIDR_PUT((__n)->len ? (__n)->nets[0].cidr : host_mask);\
})
#ifdef IP_SET_HASH_WITH_NET0
/* cidr from 0 to HOST_MASK value and c = cidr + 1 */
#define NLEN (HOST_MASK + 1)
#define CIDR_POS(c) ((c) - 1)
#else
/* cidr from 1 to HOST_MASK value and c = cidr + 1 */
#define NLEN HOST_MASK
#define CIDR_POS(c) ((c) - 2)
#endif
#else
#define NLEN 0
#endif /* IP_SET_HASH_WITH_NETS */
#define SET_ELEM_EXPIRED(set, d) \
@@ -292,6 +283,7 @@ static const union nf_inet_addr zeromask = {};
/* The generic hash structure */
struct htype {
struct htable __rcu *table; /* the hash table */
struct net_prefixes __rcu *rnets[IPSET_NET_COUNT]; /* cidr prefixes */
struct htable_gc gc; /* gc workqueue */
u32 maxelem; /* max elements in the hash */
u32 initval; /* random jhash init value */
@@ -302,9 +294,6 @@ struct htype {
#if defined(IP_SET_HASH_WITH_NETMASK) || defined(IP_SET_HASH_WITH_BITMASK)
u8 netmask; /* netmask value for subnets to store */
union nf_inet_addr bitmask; /* stores bitmask */
#endif
#ifdef IP_SET_HASH_WITH_NETS
struct net_prefixes nets[NLEN]; /* book-keeping of prefixes */
#endif
/* Because 'next' is IPv4/IPv6 dependent, no elements of this
* structure and referred in create() may come after 'next'.
@@ -326,50 +315,92 @@ struct mtype_resize_ad {
/* Network cidr size book keeping when the hash stores different
* sized networks. cidr == real cidr + 1 to support /0.
*/
static void
static int
mtype_add_cidr(struct ip_set *set, struct htype *h, u8 cidr, u8 n)
{
int i, j;
struct net_prefixes *nets, *tmp;
int i, j, found, len = 0, ret = 0;
spin_lock_bh(&set->lock);
nets = __ipset_dereference(h->rnets[n]);
/* Add in increasing prefix order, so larger cidr first */
for (i = 0, j = -1; i < NLEN && h->nets[i].cidr[n]; i++) {
if (j != -1) {
for (i = 0, found = -1; i < nets->len; i++) {
if (nets->nets[i].count)
len++;
if (found != -1) {
continue;
} else if (h->nets[i].cidr[n] < cidr) {
j = i;
} else if (h->nets[i].cidr[n] == cidr) {
h->nets[CIDR_POS(cidr)].nets[n]++;
} else if (nets->nets[i].cidr < cidr) {
found = i;
} else if (nets->nets[i].cidr == cidr) {
nets->nets[i].count++;
goto unlock;
}
}
if (j != -1) {
for (; i > j; i--)
h->nets[i].cidr[n] = h->nets[i - 1].cidr[n];
len++;
tmp = kzalloc_flex(*tmp, nets, len, GFP_ATOMIC);
if (!tmp) {
ret = -ENOMEM;
goto unlock;
}
h->nets[i].cidr[n] = cidr;
h->nets[CIDR_POS(cidr)].nets[n] = 1;
tmp->len = len;
for (i = 0, j = 0; i < nets->len; i++) {
if (i == found) {
tmp->nets[j].cidr = cidr;
tmp->nets[j++].count = 1;
}
if (!nets->nets[i].count)
continue;
tmp->nets[j].cidr = nets->nets[i].cidr;
tmp->nets[j++].count = nets->nets[i].count;
}
if (found == -1) {
tmp->nets[j].cidr = cidr;
tmp->nets[j].count = 1;
}
rcu_assign_pointer(h->rnets[n], tmp);
kfree_rcu(nets, rcu);
unlock:
spin_unlock_bh(&set->lock);
return ret;
}
static void
mtype_del_cidr(struct ip_set *set, struct htype *h, u8 cidr, u8 n)
{
u8 i, j, net_end = NLEN - 1;
struct net_prefixes *nets, *tmp;
u8 i, j, len = 0;
int found;
spin_lock_bh(&set->lock);
for (i = 0; i < NLEN; i++) {
if (h->nets[i].cidr[n] != cidr)
continue;
h->nets[CIDR_POS(cidr)].nets[n]--;
if (h->nets[CIDR_POS(cidr)].nets[n] > 0)
goto unlock;
for (j = i; j < net_end && h->nets[j].cidr[n]; j++)
h->nets[j].cidr[n] = h->nets[j + 1].cidr[n];
h->nets[j].cidr[n] = 0;
goto unlock;
nets = __ipset_dereference(h->rnets[n]);
for (i = 0, found = -1; i < nets->len; i++) {
if (nets->nets[i].count)
len++;
if (nets->nets[i].cidr == cidr)
found = i;
}
if (unlikely(found == -1))
goto unlock;
nets->nets[found].count--;
if (nets->nets[found].count)
goto unlock;
len--;
tmp = kzalloc_flex(*tmp, nets, len, GFP_ATOMIC);
if (!tmp)
/* Leave a hole */
goto unlock;
tmp->len = len;
for (i = 0, j = 0; i < nets->len; i++) {
if (!nets->nets[i].count || i == found)
continue;
tmp->nets[j].cidr = nets->nets[i].cidr;
tmp->nets[j++].count = nets->nets[i].count;
}
rcu_assign_pointer(h->rnets[n], tmp);
kfree_rcu(nets, rcu);
unlock:
spin_unlock_bh(&set->lock);
}
@@ -402,6 +433,9 @@ static void
mtype_flush(struct ip_set *set)
{
struct htype *h = set->data;
#ifdef IP_SET_HASH_WITH_NETS
struct net_prefixes *nets, *tmp;
#endif
struct htable *t;
struct hbucket *n;
u32 r, i;
@@ -425,7 +459,19 @@ mtype_flush(struct ip_set *set)
spin_unlock_bh(&t->hregion[r].lock);
}
#ifdef IP_SET_HASH_WITH_NETS
memset(h->nets, 0, sizeof(h->nets));
for (i = 0; i < IPSET_NET_COUNT; i++) {
nets = ipset_dereference_nfnl(h->rnets[i]);
tmp = kzalloc_obj(*tmp, GFP_ATOMIC);
if (!tmp) {
u8 j;
for (j = 0; j < nets->len; j++)
nets->nets[j].count = 0;
} else {
rcu_assign_pointer(h->rnets[i], tmp);
kfree_rcu(nets, rcu);
}
}
#endif
}
@@ -433,6 +479,9 @@ mtype_flush(struct ip_set *set)
static void
mtype_ahash_destroy(struct ip_set *set, struct htable *t, bool ext_destroy)
{
#ifdef IP_SET_HASH_WITH_NETS
struct htype *h = set->data;
#endif
struct hbucket *n;
u32 i;
@@ -446,6 +495,11 @@ mtype_ahash_destroy(struct ip_set *set, struct htable *t, bool ext_destroy)
kfree(n);
}
#ifdef IP_SET_HASH_WITH_NETS
if (ext_destroy)
for (i = 0; i < IPSET_NET_COUNT; i++)
kfree(rcu_dereference_raw(h->rnets[i]));
#endif
ip_set_free(t->hregion);
ip_set_free(t);
}
@@ -519,8 +573,7 @@ mtype_gc_do(struct ip_set *set, struct htype *h, struct htable *t, u32 r)
#ifdef IP_SET_HASH_WITH_NETS
for (k = 0; k < IPSET_NET_COUNT; k++)
mtype_del_cidr(set, h,
NCIDR_PUT(DCIDR_GET(data->cidr, k)),
k);
DCIDR_GET(data->cidr, k), k);
#endif
t->hregion[r].elements--;
ip_set_ext_destroy(set, data);
@@ -950,8 +1003,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
#ifdef IP_SET_HASH_WITH_NETS
for (i = 0; i < IPSET_NET_COUNT; i++)
mtype_del_cidr(set, h,
NCIDR_PUT(DCIDR_GET(data->cidr, i)),
i);
DCIDR_GET(data->cidr, i), i);
#endif
ip_set_ext_destroy(set, data);
t->hregion[r].elements--;
@@ -996,7 +1048,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
t->hregion[r].elements++;
#ifdef IP_SET_HASH_WITH_NETS
for (i = 0; i < IPSET_NET_COUNT; i++)
mtype_add_cidr(set, h, NCIDR_PUT(DCIDR_GET(d->cidr, i)), i);
mtype_add_cidr(set, h, DCIDR_GET(d->cidr, i), i);
#endif
memcpy(data, d, sizeof(struct mtype_elem));
overwrite_extensions:
@@ -1110,7 +1162,7 @@ mtype_del(struct ip_set *set, void *value, const struct ip_set_ext *ext,
#ifdef IP_SET_HASH_WITH_NETS
for (j = 0; j < IPSET_NET_COUNT; j++)
mtype_del_cidr(set, h,
NCIDR_PUT(DCIDR_GET(d->cidr, j)), j);
DCIDR_GET(d->cidr, j), j);
#endif
ip_set_ext_destroy(set, data);
@@ -1193,28 +1245,37 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
{
struct htype *h = set->data;
struct htable *t = rcu_dereference_bh(h->table);
struct net_prefixes *nets0;
struct hbucket *n;
struct mtype_elem *data;
#if IPSET_NET_COUNT == 2
struct net_prefixes *nets1;
struct mtype_elem orig = *d;
int ret, i, j = 0, k;
int ret, i, j, k;
#else
int ret, i, j = 0;
int ret, i, j;
#endif
u32 key, multi = 0;
u8 pos;
pr_debug("test by nets\n");
for (; j < NLEN && h->nets[j].cidr[0] && !multi; j++) {
rcu_read_lock_bh();
nets0 = rcu_dereference_bh(h->rnets[0]);
#if IPSET_NET_COUNT == 2
nets1 = rcu_dereference_bh(h->rnets[1]);
#endif
for (j = 0; j < nets0->len && !multi; j++) {
if (!nets0->nets[j].count)
continue;
#if IPSET_NET_COUNT == 2
mtype_data_reset_elem(d, &orig);
mtype_data_netmask(d, NCIDR_GET(h->nets[j].cidr[0]), false);
for (k = 0; k < NLEN && h->nets[k].cidr[1] && !multi;
k++) {
mtype_data_netmask(d, NCIDR_GET(h->nets[k].cidr[1]),
true);
mtype_data_netmask(d, nets0->nets[j].cidr, false);
for (k = 0; k < nets1->len && !multi; k++) {
if (!nets1->nets[k].count)
continue;
mtype_data_netmask(d, nets1->nets[k].cidr, true);
#else
mtype_data_netmask(d, NCIDR_GET(h->nets[j].cidr[0]));
mtype_data_netmask(d, nets0->nets[j].cidr);
#endif
key = HKEY(d, h->initval, t->htable_bits);
n = rcu_dereference_bh(hbucket(t, key));
@@ -1229,7 +1290,7 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
continue;
ret = mtype_data_match(data, ext, mext, set, flags);
if (ret != 0)
return ret;
goto unlock;
#ifdef IP_SET_HASH_WITH_MULTI
/* No match, reset multiple match flag */
multi = 0;
@@ -1239,7 +1300,10 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,
}
#endif
}
return 0;
ret = 0;
unlock:
rcu_read_unlock_bh();
return ret;
}
#endif
@@ -1504,6 +1568,9 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct ip_set *set,
int ret __attribute__((unused)) = 0;
u8 netmask = set->family == NFPROTO_IPV4 ? 32 : 128;
union nf_inet_addr bitmask = onesmask;
#endif
#ifdef IP_SET_HASH_WITH_NETS
struct net_prefixes *nets;
#endif
size_t hsize;
struct htype *h;
@@ -1604,21 +1671,25 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct ip_set *set,
*/
hbits = fls(hashsize - 1);
hsize = htable_size(hbits);
if (hsize == 0) {
kfree(h);
return -ENOMEM;
}
if (hsize == 0)
goto free_h;
t = ip_set_alloc(hsize);
if (!t) {
kfree(h);
return -ENOMEM;
}
if (!t)
goto free_h;
t->hregion = ip_set_alloc(ahash_sizeof_regions(hbits));
if (!t->hregion) {
ip_set_free(t);
kfree(h);
return -ENOMEM;
if (!t->hregion)
goto free_t;
#ifdef IP_SET_HASH_WITH_NETS
for (i = 0; i < IPSET_NET_COUNT; i++) {
nets = kzalloc_obj(*nets);
if (!nets) {
while (i > 0)
kfree(rcu_dereference_raw(h->rnets[--i]));
goto free_hregion;
}
RCU_INIT_POINTER(h->rnets[i], nets);
}
#endif
h->gc.set = set;
spin_lock_init(&h->gc.lock);
for (i = 0; i < ahash_numof_locks(hbits); i++)
@@ -1682,6 +1753,16 @@ IPSET_TOKEN(HTYPE, _create)(struct net *net, struct ip_set *set,
t->htable_bits, h->maxelem, set->data, t);
return 0;
#ifdef IP_SET_HASH_WITH_NETS
free_hregion:
ip_set_free(t->hregion);
#endif
free_t:
ip_set_free(t);
free_h:
kfree(h);
return -ENOMEM;
}
#endif /* IP_SET_EMIT_CREATE */

View File

@@ -138,7 +138,7 @@ hash_ipportnet4_kadt(struct ip_set *set, const struct sk_buff *skb,
const struct hash_ipportnet4 *h = set->data;
ipset_adtfn adtfn = set->variant->adt[adt];
struct hash_ipportnet4_elem e = {
.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
};
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
@@ -398,7 +398,7 @@ hash_ipportnet6_kadt(struct ip_set *set, const struct sk_buff *skb,
const struct hash_ipportnet6 *h = set->data;
ipset_adtfn adtfn = set->variant->adt[adt];
struct hash_ipportnet6_elem e = {
.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
};
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);

View File

@@ -117,7 +117,7 @@ hash_net4_kadt(struct ip_set *set, const struct sk_buff *skb,
const struct hash_net4 *h = set->data;
ipset_adtfn adtfn = set->variant->adt[adt];
struct hash_net4_elem e = {
.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
};
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
@@ -291,7 +291,7 @@ hash_net6_kadt(struct ip_set *set, const struct sk_buff *skb,
const struct hash_net6 *h = set->data;
ipset_adtfn adtfn = set->variant->adt[adt];
struct hash_net6_elem e = {
.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
};
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);

View File

@@ -161,7 +161,7 @@ hash_netiface4_kadt(struct ip_set *set, const struct sk_buff *skb,
struct hash_netiface4 *h = set->data;
ipset_adtfn adtfn = set->variant->adt[adt];
struct hash_netiface4_elem e = {
.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
.elem = 1,
};
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
@@ -382,7 +382,7 @@ hash_netiface6_kadt(struct ip_set *set, const struct sk_buff *skb,
struct hash_netiface6 *h = set->data;
ipset_adtfn adtfn = set->variant->adt[adt];
struct hash_netiface6_elem e = {
.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
.elem = 1,
};
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);

View File

@@ -149,8 +149,10 @@ hash_netnet4_kadt(struct ip_set *set, const struct sk_buff *skb,
struct hash_netnet4_elem e = { };
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
e.cidr[0] = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK);
e.cidr[1] = INIT_CIDR(h->nets[0].cidr[1], HOST_MASK);
rcu_read_lock_bh();
e.cidr[0] = INIT_CIDR(h->rnets[0], HOST_MASK);
e.cidr[1] = INIT_CIDR(h->rnets[1], HOST_MASK);
rcu_read_unlock_bh();
if (adt == IPSET_TEST)
e.ccmp = (HOST_MASK << (sizeof(e.cidr[0]) * 8)) | HOST_MASK;
@@ -388,8 +390,10 @@ hash_netnet6_kadt(struct ip_set *set, const struct sk_buff *skb,
struct hash_netnet6_elem e = { };
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
e.cidr[0] = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK);
e.cidr[1] = INIT_CIDR(h->nets[0].cidr[1], HOST_MASK);
rcu_read_lock_bh();
e.cidr[0] = INIT_CIDR(h->rnets[0], HOST_MASK);
e.cidr[1] = INIT_CIDR(h->rnets[1], HOST_MASK);
rcu_read_unlock_bh();
if (adt == IPSET_TEST)
e.ccmp = (HOST_MASK << (sizeof(u8) * 8)) | HOST_MASK;

View File

@@ -133,7 +133,7 @@ hash_netport4_kadt(struct ip_set *set, const struct sk_buff *skb,
const struct hash_netport4 *h = set->data;
ipset_adtfn adtfn = set->variant->adt[adt];
struct hash_netport4_elem e = {
.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
};
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
@@ -353,7 +353,7 @@ hash_netport6_kadt(struct ip_set *set, const struct sk_buff *skb,
const struct hash_netport6 *h = set->data;
ipset_adtfn adtfn = set->variant->adt[adt];
struct hash_netport6_elem e = {
.cidr = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK),
.cidr = INIT_CIDR(h->rnets[0], HOST_MASK),
};
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);

View File

@@ -157,8 +157,10 @@ hash_netportnet4_kadt(struct ip_set *set, const struct sk_buff *skb,
struct hash_netportnet4_elem e = { };
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
e.cidr[0] = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK);
e.cidr[1] = INIT_CIDR(h->nets[0].cidr[1], HOST_MASK);
rcu_read_lock_bh();
e.cidr[0] = INIT_CIDR(h->rnets[0], HOST_MASK);
e.cidr[1] = INIT_CIDR(h->rnets[1], HOST_MASK);
rcu_read_unlock_bh();
if (adt == IPSET_TEST)
e.ccmp = (HOST_MASK << (sizeof(e.cidr[0]) * 8)) | HOST_MASK;
@@ -452,8 +454,10 @@ hash_netportnet6_kadt(struct ip_set *set, const struct sk_buff *skb,
struct hash_netportnet6_elem e = { };
struct ip_set_ext ext = IP_SET_INIT_KEXT(skb, opt, set);
e.cidr[0] = INIT_CIDR(h->nets[0].cidr[0], HOST_MASK);
e.cidr[1] = INIT_CIDR(h->nets[0].cidr[1], HOST_MASK);
rcu_read_lock_bh();
e.cidr[0] = INIT_CIDR(h->rnets[0], HOST_MASK);
e.cidr[1] = INIT_CIDR(h->rnets[1], HOST_MASK);
rcu_read_unlock_bh();
if (adt == IPSET_TEST)
e.ccmp = (HOST_MASK << (sizeof(u8) * 8)) | HOST_MASK;