Skip to content

Commit bc2590c

Browse files
committed
netfilter: nf_tables: don't skip expired elements during walk
jira VUlN-429 subsystem-sync netfilter:nf_tables 4.18.0-511 commit-author Florian Westphal <[email protected]> commit 2413893 There is an asymmetry between commit/abort and preparation phase if the following conditions are met: 1. set is a verdict map ("1.2.3.4 : jump foo") 2. timeouts are enabled In this case, following sequence is problematic: 1. element E in set S refers to chain C 2. userspace requests removal of set S 3. kernel does a set walk to decrement chain->use count for all elements from preparation phase 4. kernel does another set walk to remove elements from the commit phase (or another walk to do a chain->use increment for all elements from abort phase) If E has already expired in 1), it will be ignored during list walk, so its use count won't have been changed. Then, when set is culled, ->destroy callback will zap the element via nf_tables_set_elem_destroy(), but this function is only safe for elements that have been deactivated earlier from the preparation phase: lack of earlier deactivate removes the element but leaks the chain use count, which results in a WARN splat when the chain gets removed later, plus a leak of the nft_chain structure. Update pipapo_get() not to skip expired elements, otherwise flush command reports bogus ENOENT errors. Fixes: 3c4287f ("nf_tables: Add set type for arbitrary concatenation of ranges") Fixes: 8d8540c ("netfilter: nft_set_rbtree: add timeout support") Fixes: 9d09829 ("netfilter: nft_hash: add support for timeouts") Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit 2413893) Signed-off-by: Greg Rose <[email protected]>
1 parent 1567d4f commit bc2590c

File tree

4 files changed

+16
-10
lines changed

4 files changed

+16
-10
lines changed

net/netfilter/nf_tables_api.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4751,8 +4751,12 @@ static int nf_tables_dump_setelem(const struct nft_ctx *ctx,
47514751
const struct nft_set_iter *iter,
47524752
struct nft_set_elem *elem)
47534753
{
4754+
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
47544755
struct nft_set_dump_args *args;
47554756

4757+
if (nft_set_elem_expired(ext))
4758+
return 0;
4759+
47564760
args = container_of(iter, struct nft_set_dump_args, iter);
47574761
return nf_tables_fill_setelem(args->skb, set, elem);
47584762
}

net/netfilter/nft_set_hash.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,6 @@ static void nft_rhash_walk(const struct nft_ctx *ctx, struct nft_set *set,
262262

263263
if (iter->count < iter->skip)
264264
goto cont;
265-
if (nft_set_elem_expired(&he->ext))
266-
goto cont;
267265
if (!nft_set_elem_active(&he->ext, iter->genmask))
268266
goto cont;
269267

net/netfilter/nft_set_pipapo.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -704,8 +704,7 @@ 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-
(genmask &&
707+
if ((genmask &&
709708
!nft_set_elem_active(&f->mt[b].e->ext, genmask)))
710709
goto next_match;
711710

@@ -739,8 +738,17 @@ static struct nft_pipapo_elem *pipapo_get(const struct net *net,
739738
void *nft_pipapo_get(const struct net *net, const struct nft_set *set,
740739
const struct nft_set_elem *elem, unsigned int flags)
741740
{
742-
return pipapo_get(net, set, (const u8 *)elem->key.val.data,
743-
nft_genmask_cur(net));
741+
struct nft_pipapo_elem *ret;
742+
743+
ret = pipapo_get(net, set, (const u8 *)elem->key.val.data,
744+
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;
744752
}
745753

746754
/**
@@ -1890,8 +1898,6 @@ static void nft_pipapo_walk(const struct nft_ctx *ctx, struct nft_set *set,
18901898
goto cont;
18911899

18921900
e = f->mt[r].e;
1893-
if (nft_set_elem_expired(&e->ext))
1894-
goto cont;
18951901

18961902
elem.priv = e;
18971903

net/netfilter/nft_set_rbtree.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,8 +446,6 @@ static void nft_rbtree_walk(const struct nft_ctx *ctx,
446446

447447
if (iter->count < iter->skip)
448448
goto cont;
449-
if (nft_set_elem_expired(&rbe->ext))
450-
goto cont;
451449
if (!nft_set_elem_active(&rbe->ext, iter->genmask))
452450
goto cont;
453451

0 commit comments

Comments
 (0)