|
 |
fc8f74 |
From 38c94a9f5ea03deffe0a34056a0f83a4af4641bb Mon Sep 17 00:00:00 2001
|
|
|
fc8f74 |
From: Phil Sutter <phil@nwl.cc>
|
|
|
fc8f74 |
Date: Fri, 13 Mar 2020 13:02:12 +0100
|
|
|
fc8f74 |
Subject: [PATCH] nft: cache: Fix iptables-save segfault under stress
|
|
|
fc8f74 |
|
|
|
fc8f74 |
If kernel ruleset is constantly changing, code called by
|
|
|
fc8f74 |
nft_is_table_compatible() may crash: For each item in table's chain
|
|
|
fc8f74 |
list, nft_is_chain_compatible() is called. This in turn calls
|
|
|
fc8f74 |
nft_build_cache() to fetch chain's rules. Though if kernel genid has changed
|
|
|
fc8f74 |
meanwhile, cache is flushed and rebuilt from scratch, thereby freeing
|
|
|
fc8f74 |
table's chain list - the foreach loop in nft_is_table_compatible() then
|
|
|
fc8f74 |
operates on freed memory.
|
|
|
fc8f74 |
|
|
|
fc8f74 |
A simple reproducer (may need a few calls):
|
|
|
fc8f74 |
|
|
|
fc8f74 |
| RULESET='*filter
|
|
|
fc8f74 |
| :INPUT ACCEPT [10517:1483527]
|
|
|
fc8f74 |
| :FORWARD ACCEPT [0:0]
|
|
|
fc8f74 |
| :OUTPUT ACCEPT [1714:105671]
|
|
|
fc8f74 |
| COMMIT
|
|
|
fc8f74 |
| '
|
|
|
fc8f74 |
|
|
|
|
fc8f74 |
| for ((i = 0; i < 100; i++)); do
|
|
|
fc8f74 |
| iptables-nft-restore <<< "$RULESET" &
|
|
|
fc8f74 |
| done &
|
|
|
fc8f74 |
| iptables-nft-save
|
|
|
fc8f74 |
|
|
|
fc8f74 |
To fix the problem, basically revert commit ab1cd3b510fa5 ("nft: ensure
|
|
|
fc8f74 |
cache consistency") so that __nft_build_cache() no longer flushes the
|
|
|
fc8f74 |
cache. Instead just record kernel's genid when fetching for the first
|
|
|
fc8f74 |
time. If kernel rule set changes until the changes are committed, the
|
|
|
fc8f74 |
commit simply fails and local cache is being rebuilt.
|
|
|
fc8f74 |
|
|
|
fc8f74 |
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
|
|
fc8f74 |
(cherry picked from commit 200bc399651499f502ac0de45f4d4aa4c9d37ab6)
|
|
|
fc8f74 |
Signed-off-by: Phil Sutter <psutter@redhat.com>
|
|
|
fc8f74 |
---
|
|
|
fc8f74 |
iptables/nft-cache.c | 16 ++--------------
|
|
|
fc8f74 |
1 file changed, 2 insertions(+), 14 deletions(-)
|
|
|
fc8f74 |
|
|
|
fc8f74 |
diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
|
|
|
fc8f74 |
index 6f21f2283e0fb..07265b7795e4f 100644
|
|
|
fc8f74 |
--- a/iptables/nft-cache.c
|
|
|
fc8f74 |
+++ b/iptables/nft-cache.c
|
|
|
fc8f74 |
@@ -452,15 +452,11 @@ __nft_build_cache(struct nft_handle *h, enum nft_cache_level level,
|
|
|
fc8f74 |
const struct builtin_table *t, const char *set,
|
|
|
fc8f74 |
const char *chain)
|
|
|
fc8f74 |
{
|
|
|
fc8f74 |
- uint32_t genid_start, genid_stop;
|
|
|
fc8f74 |
-
|
|
|
fc8f74 |
if (level <= h->cache_level)
|
|
|
fc8f74 |
return;
|
|
|
fc8f74 |
-retry:
|
|
|
fc8f74 |
- mnl_genid_get(h, &genid_start);
|
|
|
fc8f74 |
|
|
|
fc8f74 |
- if (h->cache_level && genid_start != h->nft_genid)
|
|
|
fc8f74 |
- flush_chain_cache(h, NULL);
|
|
|
fc8f74 |
+ if (!h->nft_genid)
|
|
|
fc8f74 |
+ mnl_genid_get(h, &h->nft_genid);
|
|
|
fc8f74 |
|
|
|
fc8f74 |
switch (h->cache_level) {
|
|
|
fc8f74 |
case NFT_CL_NONE:
|
|
|
fc8f74 |
@@ -487,18 +483,10 @@ retry:
|
|
|
fc8f74 |
break;
|
|
|
fc8f74 |
}
|
|
|
fc8f74 |
|
|
|
fc8f74 |
- mnl_genid_get(h, &genid_stop);
|
|
|
fc8f74 |
- if (genid_start != genid_stop) {
|
|
|
fc8f74 |
- flush_chain_cache(h, NULL);
|
|
|
fc8f74 |
- goto retry;
|
|
|
fc8f74 |
- }
|
|
|
fc8f74 |
-
|
|
|
fc8f74 |
if (!t && !chain)
|
|
|
fc8f74 |
h->cache_level = level;
|
|
|
fc8f74 |
else if (h->cache_level < NFT_CL_TABLES)
|
|
|
fc8f74 |
h->cache_level = NFT_CL_TABLES;
|
|
|
fc8f74 |
-
|
|
|
fc8f74 |
- h->nft_genid = genid_start;
|
|
|
fc8f74 |
}
|
|
|
fc8f74 |
|
|
|
fc8f74 |
void nft_build_cache(struct nft_handle *h, struct nftnl_chain *c)
|
|
|
fc8f74 |
--
|
|
|
fc8f74 |
2.25.1
|
|
|
fc8f74 |
|