netfilter: nf_tables: add nft_set_elem_expr_destroy() and use it
authorPablo Neira Ayuso <pablo@netfilter.org>
Wed, 18 Mar 2020 13:29:45 +0000 (14:29 +0100)
committerPablo Neira Ayuso <pablo@netfilter.org>
Thu, 19 Mar 2020 10:37:32 +0000 (11:37 +0100)
This patch adds nft_set_elem_expr_destroy() to destroy stateful
expressions in set elements.

This patch also updates the commit path to call this function to invoke
expr->ops->destroy_clone when required.

This is implicitly fixing up a module reference counter leak and
a memory leak in expressions that allocated internal state, e.g.
nft_counter.

Fixes: 409444522976 ("netfilter: nf_tables: add elements with stateful expressions")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
net/netfilter/nf_tables_api.c

index 29ad33e..c5332a3 100644 (file)
@@ -4882,6 +4882,17 @@ void *nft_set_elem_init(const struct nft_set *set,
        return elem;
 }
 
+static void nft_set_elem_expr_destroy(const struct nft_ctx *ctx,
+                                     struct nft_expr *expr)
+{
+       if (expr->ops->destroy_clone) {
+               expr->ops->destroy_clone(ctx, expr);
+               module_put(expr->ops->type->owner);
+       } else {
+               nf_tables_expr_destroy(ctx, expr);
+       }
+}
+
 void nft_set_elem_destroy(const struct nft_set *set, void *elem,
                          bool destroy_expr)
 {
@@ -4894,16 +4905,9 @@ void nft_set_elem_destroy(const struct nft_set *set, void *elem,
        nft_data_release(nft_set_ext_key(ext), NFT_DATA_VALUE);
        if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA))
                nft_data_release(nft_set_ext_data(ext), set->dtype);
-       if (destroy_expr && nft_set_ext_exists(ext, NFT_SET_EXT_EXPR)) {
-               struct nft_expr *expr = nft_set_ext_expr(ext);
+       if (destroy_expr && nft_set_ext_exists(ext, NFT_SET_EXT_EXPR))
+               nft_set_elem_expr_destroy(&ctx, nft_set_ext_expr(ext));
 
-               if (expr->ops->destroy_clone) {
-                       expr->ops->destroy_clone(&ctx, expr);
-                       module_put(expr->ops->type->owner);
-               } else {
-                       nf_tables_expr_destroy(&ctx, expr);
-               }
-       }
        if (nft_set_ext_exists(ext, NFT_SET_EXT_OBJREF))
                (*nft_set_ext_obj(ext))->use--;
        kfree(elem);
@@ -4919,7 +4923,8 @@ static void nf_tables_set_elem_destroy(const struct nft_ctx *ctx,
        struct nft_set_ext *ext = nft_set_elem_ext(set, elem);
 
        if (nft_set_ext_exists(ext, NFT_SET_EXT_EXPR))
-               nf_tables_expr_destroy(ctx, nft_set_ext_expr(ext));
+               nft_set_elem_expr_destroy(ctx, nft_set_ext_expr(ext));
+
        kfree(elem);
 }
 
@@ -5182,7 +5187,8 @@ err_element_clash:
 err_trans:
        if (obj)
                obj->use--;
-       kfree(elem.priv);
+
+       nf_tables_set_elem_destroy(ctx, set, elem.priv);
 err_parse_data:
        if (nla[NFTA_SET_ELEM_DATA] != NULL)
                nft_data_release(&data, desc.type);