Skip to content

Commit 9cac933

Browse files
committed
netfilter: nf_tables: don't fail inserts if duplicate has expired
jira VUlN-597 subsystem-sync netfilter:nf_tables 4.18.0-534 commit-author Florian Westphal <fw@strlen.de> commit 7845914 nftables selftests fail: run-tests.sh testcases/sets/0044interval_overlap_0 Expected: 0-2 . 0-3, got: W: [FAILED] ./testcases/sets/0044interval_overlap_0: got 1 Insertion must ignore duplicate but expired entries. Moreover, there is a strange asymmetry in nft_pipapo_activate: It refetches the current element, whereas the other ->activate callbacks (bitmap, hash, rhash, rbtree) use elem->priv. Same for .remove: other set implementations take elem->priv, nft_pipapo_remove fetches elem->priv, then does a relookup, remove this. I suspect this was the reason for the change that prompted the removal of the expired check in pipapo_get() in the first place, but skipping exired elements there makes no sense to me, this helper is used for normal get requests, insertions (duplicate check) and deactivate callback. In first two cases expired elements must be skipped. For ->deactivate(), this gets called for DELSETELEM, so it seems to me that expired elements should be skipped as well, i.e. delete request should fail with -ENOENT error. Fixes: 2413893 ("netfilter: nf_tables: don't skip expired elements during walk") Signed-off-by: Florian Westphal <fw@strlen.de> (cherry picked from commit 7845914) Signed-off-by: Greg Rose <g.v.rose@ciq.com>
1 parent 211fb1b commit 9cac933

File tree

1 file changed

+4
-19
lines changed

1 file changed

+4
-19
lines changed

net/netfilter/nft_set_pipapo.c

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,8 @@ static struct nft_pipapo_elem *pipapo_get(const struct net *net,
704704
goto out;
705705

706706
if (last) {
707+
if (nft_set_elem_expired(&f->mt[b].e->ext))
708+
goto next_match;
707709
if ((genmask &&
708710
!nft_set_elem_active(&f->mt[b].e->ext, genmask)))
709711
goto next_match;
@@ -738,17 +740,8 @@ static struct nft_pipapo_elem *pipapo_get(const struct net *net,
738740
void *nft_pipapo_get(const struct net *net, const struct nft_set *set,
739741
const struct nft_set_elem *elem, unsigned int flags)
740742
{
741-
struct nft_pipapo_elem *ret;
742-
743-
ret = pipapo_get(net, set, (const u8 *)elem->key.val.data,
743+
return pipapo_get(net, set, (const u8 *)elem->key.val.data,
744744
nft_genmask_cur(net));
745-
if (IS_ERR(ret))
746-
return ret;
747-
748-
if (nft_set_elem_expired(&ret->ext))
749-
return ERR_PTR(-ENOENT);
750-
751-
return ret;
752745
}
753746

754747
/**
@@ -1639,11 +1632,7 @@ static void nft_pipapo_activate(const struct net *net,
16391632
const struct nft_set *set,
16401633
const struct nft_set_elem *elem)
16411634
{
1642-
struct nft_pipapo_elem *e;
1643-
1644-
e = pipapo_get(net, set, (const u8 *)elem->key.val.data, 0);
1645-
if (IS_ERR(e))
1646-
return;
1635+
struct nft_pipapo_elem *e = elem->priv;
16471636

16481637
nft_set_elem_change_active(net, set, &e->ext);
16491638
}
@@ -1853,10 +1842,6 @@ static void nft_pipapo_remove(const struct net *net, const struct nft_set *set,
18531842

18541843
data = (const u8 *)nft_set_ext_key(&e->ext);
18551844

1856-
e = pipapo_get(net, set, data, 0);
1857-
if (IS_ERR(e))
1858-
return;
1859-
18601845
while ((rules_f0 = pipapo_rules_same_key(m->f, first_rule))) {
18611846
union nft_pipapo_map_bucket rulemap[NFT_PIPAPO_MAX_FIELDS];
18621847
const u8 *match_start, *match_end;

0 commit comments

Comments
 (0)