ruleset: fix crash if we free sets included in the set_list

When we parse a ruleset which has a rule using a set. First step is to
parse the set, set up an ID and add it to a set list. Later, we use this
set list to find the set associated to the rule and we set up the set ID
to the expression (lookup expression) of the rule.

The problem is that if we return this set to the callback function
nft_ruleset_parse_file_cb() and we free this set, we have a crash when
we try to iterate in the set list.

This patch solves it, cloning the set and adding the new set to the set
list.

Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/libnftnl/set.h b/include/libnftnl/set.h
index 7f3504f43025c00a3f695f68bb928be0df95268f..55a47b0f37a02252a958b62fcb1d53f38a15bbd0 100644 (file)
--- a/include/libnftnl/set.h
+++ b/include/libnftnl/set.h
@@ -29,6 +29,8 @@ struct nft_set;
 struct nft_set *nft_set_alloc(void);
 void nft_set_free(struct nft_set *s);
 
+struct nft_set *nft_set_clone(const struct nft_set *set);
+
 bool nft_set_attr_is_set(const struct nft_set *s, uint16_t attr);
 void nft_set_attr_unset(struct nft_set *s, uint16_t attr);
 void nft_set_attr_set(struct nft_set *s, uint16_t attr, const void *data);
@@ -91,6 +93,8 @@ struct nft_set_elem;
 struct nft_set_elem *nft_set_elem_alloc(void);
 void nft_set_elem_free(struct nft_set_elem *s);
 
+struct nft_set_elem *nft_set_elem_clone(struct nft_set_elem *elem);
+
 void nft_set_elem_add(struct nft_set *s, struct nft_set_elem *elem);
 
 void nft_set_elem_attr_unset(struct nft_set_elem *s, uint16_t attr);

diff --git a/src/ruleset.c b/src/ruleset.c
index 89ea3440123ff1cb52e28f4fdf1ea587c3ef2798..280f1bc5872b93f7c4e56a29362f2c29bbc4fe1d 100644 (file)
--- a/src/ruleset.c
+++ b/src/ruleset.c
@@ -312,8 +312,15 @@ static int nft_ruleset_parse_set(struct nft_parse_ctx *ctx,
                                 struct nft_set *set, uint32_t type,
                                 struct nft_parse_err *err)
 {
+       struct nft_set *newset;
+
        nft_set_attr_set_u32(set, NFT_SET_ATTR_ID, ctx->set_id++);
-       nft_set_list_add_tail(set, ctx->set_list);
+
+       newset = nft_set_clone(set);
+       if (newset == NULL)
+               goto err;
+
+       nft_set_list_add_tail(newset, ctx->set_list);
 
        nft_ruleset_ctx_set_u32(ctx, NFT_RULESET_CTX_TYPE, type);
        nft_ruleset_ctx_set(ctx, NFT_RULESET_CTX_SET, set);

diff --git a/src/set.c b/src/set.c
index f810fce029389a13e30bb6f4dc0e26c174a2367a..70b4bc64bcd095bbbdb6bba11963eb615c482e8d 100644 (file)
--- a/src/set.c
+++ b/src/set.c
@@ -246,6 +246,37 @@ uint32_t nft_set_attr_get_u32(struct nft_set *s, uint16_t attr)
 }
 EXPORT_SYMBOL(nft_set_attr_get_u32);
 
+struct nft_set *nft_set_clone(const struct nft_set *set)
+{
+       struct nft_set *newset;
+       struct nft_set_elem *elem, *newelem;
+
+       newset = nft_set_alloc();
+       if (newset == NULL)
+               return NULL;
+
+       memcpy(newset, set, sizeof(*set));
+
+       if (set->flags & (1 << NFT_SET_ATTR_TABLE))
+               newset->table = strdup(set->table);
+       if (set->flags & (1 << NFT_SET_ATTR_NAME))
+               newset->name = strdup(set->name);
+
+       INIT_LIST_HEAD(&newset->element_list);
+       list_for_each_entry(elem, &set->element_list, head) {
+               newelem = nft_set_elem_clone(elem);
+               if (newelem == NULL)
+                       goto err;
+
+               list_add_tail(&newelem->head, &newset->element_list);
+       }
+
+       return newset;
+err:
+       nft_set_free(newset);
+       return NULL;
+}
+
 static void
 nft_set_nlmsg_build_desc_payload(struct nlmsghdr *nlh, struct nft_set *s)
 {

diff --git a/src/set_elem.c b/src/set_elem.c
index 25cd95126555f6e8e95dba41c7ef92e99bf4b9eb..e822acc91f7814d9613cceee724efd8066382c28 100644 (file)
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -161,6 +161,22 @@ uint32_t nft_set_elem_attr_get_u32(struct nft_set_elem *s, uint16_t attr)
 }
 EXPORT_SYMBOL(nft_set_elem_attr_get_u32);
 
+struct nft_set_elem *nft_set_elem_clone(struct nft_set_elem *elem)
+{
+       struct nft_set_elem *newelem;
+
+       newelem = nft_set_elem_alloc();
+       if (newelem == NULL)
+               return NULL;
+
+       memcpy(newelem, elem, sizeof(*elem));
+
+       if (elem->flags & (1 << NFT_SET_ELEM_ATTR_CHAIN))
+               newelem->data.chain = strdup(elem->data.chain);
+
+       return newelem;
+}
+
 void nft_set_elem_nlmsg_build_payload(struct nlmsghdr *nlh,
                                      struct nft_set_elem *e)
 {