* [PATCH iptables 1/6] nft: keep original cache in case of ERESTART
2019-05-20 19:08 [PATCH iptables 0/6] ERESTART fixes Pablo Neira Ayuso
@ 2019-05-20 19:08 ` Pablo Neira Ayuso
2019-05-20 19:08 ` [PATCH iptables 2/6] xtables: Fix for explicit rule flushes Pablo Neira Ayuso
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2019-05-20 19:08 UTC (permalink / raw)
To: netfilter-devel; +Cc: phil, fw
Phil Sutter says:
"The problem is that data in h->obj_list potentially sits in cache, too.
At least rules have to be there so insert with index works correctly. If
the cache is flushed before regenerating the batch, use-after-free
occurs which crashes the program."
This patch keeps around the original cache until we have refreshed the
batch.
Fixes: 862818ac3a0de ("xtables: add and use nft_build_cache")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
iptables/nft.c | 23 ++++++++++++++++++++---
iptables/nft.h | 3 ++-
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/iptables/nft.c b/iptables/nft.c
index 172beec9ae27..288ada4af3ca 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -811,7 +811,7 @@ int nft_init(struct nft_handle *h, const struct builtin_table *t)
h->portid = mnl_socket_get_portid(h->nl);
h->tables = t;
- h->cache = &h->__cache;
+ h->cache = &h->__cache[0];
INIT_LIST_HEAD(&h->obj_list);
INIT_LIST_HEAD(&h->err_list);
@@ -1618,14 +1618,30 @@ void nft_build_cache(struct nft_handle *h)
__nft_build_cache(h);
}
-static void nft_rebuild_cache(struct nft_handle *h)
+static void __nft_flush_cache(struct nft_handle *h)
{
- if (!h->have_cache)
+ if (!h->cache_index) {
+ h->cache_index++;
+ h->cache = &h->__cache[h->cache_index];
+ } else {
flush_chain_cache(h, NULL);
+ }
+}
+
+static void nft_rebuild_cache(struct nft_handle *h)
+{
+ if (h->have_cache)
+ __nft_flush_cache(h);
__nft_build_cache(h);
}
+static void nft_release_cache(struct nft_handle *h)
+{
+ if (h->cache_index)
+ flush_cache(&h->__cache[0], h->tables, NULL);
+}
+
struct nftnl_chain_list *nft_chain_list_get(struct nft_handle *h,
const char *table)
{
@@ -2957,6 +2973,7 @@ retry:
batch_obj_del(h, n);
}
+ nft_release_cache(h);
mnl_batch_reset(h->batch);
if (i)
diff --git a/iptables/nft.h b/iptables/nft.h
index dc0797d302b8..43eb8a39dd9c 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -48,7 +48,8 @@ struct nft_handle {
struct list_head err_list;
struct nft_family_ops *ops;
const struct builtin_table *tables;
- struct nft_cache __cache;
+ unsigned int cache_index;
+ struct nft_cache __cache[2];
struct nft_cache *cache;
bool have_cache;
bool restore;
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH iptables 2/6] xtables: Fix for explicit rule flushes
2019-05-20 19:08 [PATCH iptables 0/6] ERESTART fixes Pablo Neira Ayuso
2019-05-20 19:08 ` [PATCH iptables 1/6] nft: keep original cache in case of ERESTART Pablo Neira Ayuso
@ 2019-05-20 19:08 ` Pablo Neira Ayuso
2019-05-20 19:08 ` [PATCH iptables 3/6] nft: don't skip table addition from ERESTART Pablo Neira Ayuso
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2019-05-20 19:08 UTC (permalink / raw)
To: netfilter-devel; +Cc: phil, fw
From: Phil Sutter <phil@nwl.cc>
The commit this fixes added a new parameter to __nft_rule_flush() to
mark a rule flush job as implicit or not. Yet the code added to that
function ignores the parameter and instead always sets batch job's
'implicit' flag to 1.
Fixes: 77e6a93d5c9dc ("xtables: add and set "implict" flag on transaction objects")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
iptables/nft.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/iptables/nft.c b/iptables/nft.c
index 288ada4af3ca..b9268b63c86d 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1778,7 +1778,7 @@ __nft_rule_flush(struct nft_handle *h, const char *table,
return;
}
- obj->implicit = 1;
+ obj->implicit = implicit;
}
int nft_rule_flush(struct nft_handle *h, const char *chain, const char *table,
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH iptables 3/6] nft: don't skip table addition from ERESTART
2019-05-20 19:08 [PATCH iptables 0/6] ERESTART fixes Pablo Neira Ayuso
2019-05-20 19:08 ` [PATCH iptables 1/6] nft: keep original cache in case of ERESTART Pablo Neira Ayuso
2019-05-20 19:08 ` [PATCH iptables 2/6] xtables: Fix for explicit rule flushes Pablo Neira Ayuso
@ 2019-05-20 19:08 ` Pablo Neira Ayuso
2019-05-20 19:08 ` [PATCH iptables 4/6] nft: don't care about previous state in ERESTART Pablo Neira Ayuso
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2019-05-20 19:08 UTC (permalink / raw)
To: netfilter-devel; +Cc: phil, fw
I don't find a scenario that trigger this case.
Fixes: 58d7de0181f6 ("xtables: handle concurrent ruleset modifications")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
iptables/nft.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/iptables/nft.c b/iptables/nft.c
index b9268b63c86d..43b9153c2d58 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -2794,15 +2794,6 @@ static void nft_refresh_transaction(struct nft_handle *h)
else if (!n->skip && !exists)
n->skip = 1;
break;
- case NFT_COMPAT_TABLE_ADD:
- tablename = nftnl_table_get_str(n->table, NFTNL_TABLE_NAME);
- if (!tablename)
- continue;
-
- exists = nft_table_find(h, tablename);
- if (n->skip && !exists)
- n->skip = 0;
- break;
case NFT_COMPAT_CHAIN_USER_ADD:
tablename = nftnl_chain_get_str(n->chain, NFTNL_CHAIN_TABLE);
if (!tablename)
@@ -2822,6 +2813,7 @@ static void nft_refresh_transaction(struct nft_handle *h)
n->skip = 0;
}
break;
+ case NFT_COMPAT_TABLE_ADD:
case NFT_COMPAT_CHAIN_ADD:
case NFT_COMPAT_CHAIN_ZERO:
case NFT_COMPAT_CHAIN_USER_DEL:
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH iptables 4/6] nft: don't care about previous state in ERESTART
2019-05-20 19:08 [PATCH iptables 0/6] ERESTART fixes Pablo Neira Ayuso
` (2 preceding siblings ...)
2019-05-20 19:08 ` [PATCH iptables 3/6] nft: don't skip table addition from ERESTART Pablo Neira Ayuso
@ 2019-05-20 19:08 ` Pablo Neira Ayuso
2019-05-20 19:08 ` [PATCH iptables 5/6] nft: do not retry on EINTR Pablo Neira Ayuso
2019-05-20 19:08 ` [PATCH iptables 6/6] nft: reset netlink sender buffer size of socket restart Pablo Neira Ayuso
5 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2019-05-20 19:08 UTC (permalink / raw)
To: netfilter-devel; +Cc: phil, fw
We need to re-evalute based on the existing cache generation.
Fixes: 58d7de0181f6 ("xtables: handle concurrent ruleset modifications")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
iptables/nft.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/iptables/nft.c b/iptables/nft.c
index 43b9153c2d58..f6d407029892 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -2789,9 +2789,9 @@ static void nft_refresh_transaction(struct nft_handle *h)
if (!tablename)
continue;
exists = nft_table_find(h, tablename);
- if (n->skip && exists)
+ if (exists)
n->skip = 0;
- else if (!n->skip && !exists)
+ else
n->skip = 1;
break;
case NFT_COMPAT_CHAIN_USER_ADD:
@@ -2803,13 +2803,16 @@ static void nft_refresh_transaction(struct nft_handle *h)
if (!chainname)
continue;
+ if (!h->noflush)
+ break;
+
c = nft_chain_find(h, tablename, chainname);
- if (c && !n->skip) {
+ if (c) {
/* -restore -n flushes existing rules from redefined user-chain */
- if (h->noflush)
- __nft_rule_flush(h, tablename,
- chainname, false, true);
- } else if (!c && n->skip) {
+ __nft_rule_flush(h, tablename,
+ chainname, false, true);
+ n->skip = 1;
+ } else if (!c) {
n->skip = 0;
}
break;
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH iptables 5/6] nft: do not retry on EINTR
2019-05-20 19:08 [PATCH iptables 0/6] ERESTART fixes Pablo Neira Ayuso
` (3 preceding siblings ...)
2019-05-20 19:08 ` [PATCH iptables 4/6] nft: don't care about previous state in ERESTART Pablo Neira Ayuso
@ 2019-05-20 19:08 ` Pablo Neira Ayuso
2019-05-20 19:08 ` [PATCH iptables 6/6] nft: reset netlink sender buffer size of socket restart Pablo Neira Ayuso
5 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2019-05-20 19:08 UTC (permalink / raw)
To: netfilter-devel; +Cc: phil, fw
Patch ab1cd3b510fa ("nft: ensure cache consistency") already handles
consistency via generation ID.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
iptables/nft.c | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/iptables/nft.c b/iptables/nft.c
index f6d407029892..9a3e9fdf4f12 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1383,7 +1383,6 @@ static int fetch_table_cache(struct nft_handle *h)
struct nftnl_table_list *list;
int ret;
-retry:
list = nftnl_table_list_alloc();
if (list == NULL)
return 0;
@@ -1392,11 +1391,9 @@ retry:
NLM_F_DUMP, h->seq);
ret = mnl_talk(h, nlh, nftnl_table_list_cb, list);
- if (ret < 0 && errno == EINTR) {
+ if (ret < 0 && errno == EINTR)
assert(nft_restart(h) >= 0);
- nftnl_table_list_free(list);
- goto retry;
- }
+
h->cache->tables = list;
return 1;
@@ -1408,7 +1405,6 @@ static int fetch_chain_cache(struct nft_handle *h)
struct nlmsghdr *nlh;
int i, ret;
-retry:
fetch_table_cache(h);
for (i = 0; i < NFT_TABLE_MAX; i++) {
@@ -1426,11 +1422,8 @@ retry:
NLM_F_DUMP, h->seq);
ret = mnl_talk(h, nlh, nftnl_chain_list_cb, h);
- if (ret < 0 && errno == EINTR) {
+ if (ret < 0 && errno == EINTR)
assert(nft_restart(h) >= 0);
- flush_chain_cache(h, NULL);
- goto retry;
- }
return ret;
}
@@ -1551,22 +1544,13 @@ static int nft_rule_list_update(struct nftnl_chain *c, void *data)
nftnl_rule_set_str(rule, NFTNL_RULE_CHAIN,
nftnl_chain_get_str(c, NFTNL_CHAIN_NAME));
-retry:
nlh = nftnl_rule_nlmsg_build_hdr(buf, NFT_MSG_GETRULE, h->family,
NLM_F_DUMP, h->seq);
nftnl_rule_nlmsg_build_payload(nlh, rule);
ret = mnl_talk(h, nlh, nftnl_rule_list_cb, c);
- if (ret < 0) {
- flush_rule_cache(c);
-
- if (errno == EINTR) {
- assert(nft_restart(h) >= 0);
- goto retry;
- }
- nftnl_rule_free(rule);
- return -1;
- }
+ if (ret < 0 && errno == EINTR)
+ assert(nft_restart(h) >= 0);
nftnl_rule_free(rule);
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH iptables 6/6] nft: reset netlink sender buffer size of socket restart
2019-05-20 19:08 [PATCH iptables 0/6] ERESTART fixes Pablo Neira Ayuso
` (4 preceding siblings ...)
2019-05-20 19:08 ` [PATCH iptables 5/6] nft: do not retry on EINTR Pablo Neira Ayuso
@ 2019-05-20 19:08 ` Pablo Neira Ayuso
5 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2019-05-20 19:08 UTC (permalink / raw)
To: netfilter-devel; +Cc: phil, fw
Otherwise, mnl_set_sndbuffer() skips the buffer update after socket
restart. Then, sendmsg() fails with EMSGSIZE later on when sending the
batch to the kernel.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
iptables/nft.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/iptables/nft.c b/iptables/nft.c
index 9a3e9fdf4f12..2c61521455de 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -794,6 +794,7 @@ static int nft_restart(struct nft_handle *h)
return -1;
h->portid = mnl_socket_get_portid(h->nl);
+ nlbuffsiz = 0;
return 0;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread