src: fix memory leak in anon chain error handling

chain_stmt_destroy is called from bison destructor, but it turns out
this function won't free the associated chain.

There is no memory leak when bison can parse the input because the chain
statement evaluation step queues the embedded anon chain via cmd_alloc.
Then, a later cmd_free() releases the chain and the embedded statements.

In case of a parser error, the evaluation step is never reached and the
chain object leaks, e.g. in

  foo bar jump { return }

Bison calls the right destructor but the anonon chain and all
statements/expressions in it are not released:

HEAP SUMMARY:
    in use at exit: 1,136 bytes in 4 blocks
  total heap usage: 98 allocs, 94 frees, 840,255 bytes allocated

1,136 (568 direct, 568 indirect) bytes in 1 blocks are definitely lost in loss record 4 of 4
   at: calloc (vg_replace_malloc.c:1675)
   by: xzalloc (in libnftables.so.1.1.0)
   by: chain_alloc (in libnftables.so.1.1.0)
   by: nft_parse (in libnftables.so.1.1.0)
   by: __nft_run_cmd_from_filename (in libnftables.so.1.1.0)
   by: nft_run_cmd_from_filename (in libnftables.so.1.1.0)

To resolve this, make chain_stmt_destroy also release the embedded
chain.  This in turn requires chain refcount increases whenever a chain
is assocated with a chain statement, else we get double-free of the
chain.

Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/src/evaluate.c b/src/evaluate.c
index 1696f2b8a855927174b1bde6bb27b99fe67489f2..aaeb7b4e18d4294be7c97dd5bc36e4774e0c8773 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4596,7 +4596,7 @@ static int rule_evaluate(struct eval_ctx *ctx, struct rule *rule,
 
 static int stmt_evaluate_chain(struct eval_ctx *ctx, struct stmt *stmt)
 {
-       struct chain *chain = stmt->chain.chain;
+       struct chain *chain = chain_get(stmt->chain.chain);
        struct cmd *cmd;
 
        chain->flags |= CHAIN_F_BINDING;

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index b4d4a3da3b37522f3fb5fd910e6aad973c07e97f..b97962a30ca24691afafd5041eacc5bbd0fb8d1d 100644 (file)
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -235,7 +235,7 @@ static void netlink_parse_chain_verdict(struct netlink_parse_ctx *ctx,
        }
 
        if (chain) {
-               ctx->stmt = chain_stmt_alloc(loc, chain, verdict);
+               ctx->stmt = chain_stmt_alloc(loc, chain_get(chain), verdict);
                expr_free(expr);
        } else {
                ctx->stmt = verdict_stmt_alloc(loc, expr);

diff --git a/src/statement.c b/src/statement.c
index 695b57a6cc650aa57172f725279e8908c4b74e91..2bfed4ac982fc15ddb27fc51ccc1e43473083c6a 100644 (file)
--- a/src/statement.c
+++ b/src/statement.c
@@ -140,6 +140,7 @@ static void chain_stmt_print(const struct stmt *stmt, struct output_ctx *octx)
 static void chain_stmt_destroy(struct stmt *stmt)
 {
        expr_free(stmt->chain.expr);
+       chain_free(stmt->chain.chain);
 }
 
 static const struct stmt_ops chain_stmt_ops = {

diff --git a/tests/shell/testcases/bogons/nft-f/rule-parse-error-with-anon-chain-leak b/tests/shell/testcases/bogons/nft-f/rule-parse-error-with-anon-chain-leak
new file mode 100644 (file)
index 0000000..03a0df3
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/rule-parse-error-with-anon-chain-leak
@@ -0,0 +1,8 @@
+table inet x {
+       chain c {
+               type filter hook input priority filter; policy accept;
+               foo bar jump {
+                       return
+               }
+       }
+}