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 the old cache around until we have refreshed the batch.

Fixes: 862818ac3a0de ("xtables: add and use nft_build_cache")
Signed-off-by: Pablo Neira Ayuso <pa...@netfilter.org>
---
@Phil: I'd like to avoid the refcount infrastructure in libnftnl.

Compile tested-only.

 iptables/nft.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 14141bb7dbcf..51f05b6a6267 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -798,7 +798,10 @@ static int nft_restart(struct nft_handle *h)
        return 0;
 }
 
-static struct nft_cache cache;
+struct {
+       struct nft_cache        cache[2];
+       unsigned int            index;
+} pool;
 
 int nft_init(struct nft_handle *h, const struct builtin_table *t)
 {
@@ -813,7 +816,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 = &cache;
+       h->cache = &pool.cache[0];
 
        INIT_LIST_HEAD(&h->obj_list);
        INIT_LIST_HEAD(&h->err_list);
@@ -1555,12 +1558,25 @@ void nft_build_cache(struct nft_handle *h)
                __nft_build_cache(h);
 }
 
-static void nft_rebuild_cache(struct nft_handle *h)
+static struct nft_cache *nft_get_cache_next(void)
 {
-       if (!h->have_cache)
-               flush_chain_cache(h, NULL);
+       if (++pool.index > 1)
+               pool.index = 0;
 
+       return &pool.cache[pool.index];
+}
+
+static struct nft_cache *nft_rebuild_cache(struct nft_handle *h)
+{
+       struct nft_cache *cache = NULL;
+
+       if (h->have_cache) {
+               cache = h->cache;
+               h->cache = nft_get_cache_next();
+       }
        __nft_build_cache(h);
+
+       return cache;
 }
 
 struct nftnl_chain_list *nft_chain_list_get(struct nft_handle *h,
@@ -2902,10 +2918,13 @@ retry:
        errno = 0;
        ret = mnl_batch_talk(h->nl, h->batch, &h->err_list);
        if (ret && errno == ERESTART) {
-               nft_rebuild_cache(h);
+               struct nft_cache *old_cache = nft_rebuild_cache(h);
 
                nft_refresh_transaction(h);
 
+               if (old_cache)
+                       flush_cache(old_cache, h->tables, NULL);
+
                i=0;
                list_for_each_entry_safe(err, ne, &h->err_list, head)
                        mnl_err_list_free(err);
-- 
2.11.0

Reply via email to