| Message ID | 1423586774-20383-2-git-send-email-alvaroneay@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
On Tue, Feb 10, 2015 at 05:46:14PM +0100, Alvaro Neira Ayuso wrote: > When we parse the sets in a ruleset, we set up a id. To do that, we use > a set list. We alloc this set list and we need to free this set list. And we > don't do that. If we import a ruleset, valgrind shows: Please, remove this overelaborated description. The valgrind spot is sufficiente. More comments below: > ==18632== 285 (16 direct, 269 indirect) bytes in 1 blocks are definitely lost in > loss record 6 of 6 > ==18632== at 0x4C272B8: calloc (vg_replace_malloc.c:566) > ==18632== by 0x5043822: nft_set_list_alloc (set.c:977) > ==18632== by 0x5045483: nft_ruleset_json_parse (ruleset.c:442) > ==18632== by 0x50458BE: nft_ruleset_do_parse (ruleset.c:696) > ==18632== by 0x408AEC: do_command (rule.c:1317) > ==18632== by 0x406B05: nft_run (main.c:194) > ==18632== by 0x40667C: main (main.c:360) > > With this changes, we alloc the set list when we create the nft_parse_ctx and > free it later. > > Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com> > --- > src/ruleset.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/src/ruleset.c b/src/ruleset.c > index 15e84cf..3ce6ccc 100644 > --- a/src/ruleset.c > +++ b/src/ruleset.c > @@ -439,10 +439,6 @@ static int nft_ruleset_json_parse_ruleset(struct nft_parse_ctx *ctx, > json_t *node, *array = ctx->json; > int len, i, ret; > > - ctx->set_list = nft_set_list_alloc(); > - if (ctx->set_list == NULL) > - return -1; > - > len = json_array_size(array); > for (i = 0; i < len; i++) { > node = json_array_get(array, i); > @@ -525,6 +521,10 @@ static int nft_ruleset_json_parse(const void *json, > ctx.cb = cb; > ctx.format = type; > > + ctx.set_list = nft_set_list_alloc(); > + if (ctx.set_list == NULL) > + return -1; > + > if (arg != NULL) > nft_ruleset_ctx_set(&ctx, NFT_RULESET_CTX_DATA, arg); If you look in the ruleset.c file, now you add leaks in the error path of nft_ruleset_json_parse() after this change. Please, fix leaks all at once. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/src/ruleset.c b/src/ruleset.c index 15e84cf..3ce6ccc 100644 --- a/src/ruleset.c +++ b/src/ruleset.c @@ -439,10 +439,6 @@ static int nft_ruleset_json_parse_ruleset(struct nft_parse_ctx *ctx, json_t *node, *array = ctx->json; int len, i, ret; - ctx->set_list = nft_set_list_alloc(); - if (ctx->set_list == NULL) - return -1; - len = json_array_size(array); for (i = 0; i < len; i++) { node = json_array_get(array, i); @@ -525,6 +521,10 @@ static int nft_ruleset_json_parse(const void *json, ctx.cb = cb; ctx.format = type; + ctx.set_list = nft_set_list_alloc(); + if (ctx.set_list == NULL) + return -1; + if (arg != NULL) nft_ruleset_ctx_set(&ctx, NFT_RULESET_CTX_DATA, arg); @@ -554,6 +554,7 @@ static int nft_ruleset_json_parse(const void *json, goto err; } + nft_set_list_free(ctx.set_list); nft_jansson_free_root(root); return 0; err: @@ -573,10 +574,6 @@ static int nft_ruleset_xml_parse_ruleset(struct nft_parse_ctx *ctx, mxml_node_t *node, *array = ctx->xml; int len = 0, ret; - ctx->set_list = nft_set_list_alloc(); - if (ctx->set_list == NULL) - return -1; - for (node = mxmlFindElement(array, array, NULL, NULL, NULL, MXML_DESCEND_FIRST); node != NULL; @@ -653,6 +650,10 @@ static int nft_ruleset_xml_parse(const void *xml, struct nft_parse_err *err, ctx.cb = cb; ctx.format = type; + ctx.set_list = nft_set_list_alloc(); + if (ctx.set_list == NULL) + return -1; + if (arg != NULL) nft_ruleset_ctx_set(&ctx, NFT_RULESET_CTX_DATA, arg); @@ -670,6 +671,7 @@ static int nft_ruleset_xml_parse(const void *xml, struct nft_parse_err *err, nodecmd = mxmlWalkNext(tree, tree, MXML_NO_DESCEND); } + nft_set_list_free(ctx.set_list); mxmlDelete(tree); return 0; err:
When we parse the sets in a ruleset, we set up a id. To do that, we use a set list. We alloc this set list and we need to free this set list. And we don't do that. If we import a ruleset, valgrind shows: ==18632== 285 (16 direct, 269 indirect) bytes in 1 blocks are definitely lost in loss record 6 of 6 ==18632== at 0x4C272B8: calloc (vg_replace_malloc.c:566) ==18632== by 0x5043822: nft_set_list_alloc (set.c:977) ==18632== by 0x5045483: nft_ruleset_json_parse (ruleset.c:442) ==18632== by 0x50458BE: nft_ruleset_do_parse (ruleset.c:696) ==18632== by 0x408AEC: do_command (rule.c:1317) ==18632== by 0x406B05: nft_run (main.c:194) ==18632== by 0x40667C: main (main.c:360) With this changes, we alloc the set list when we create the nft_parse_ctx and free it later. Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com> --- src/ruleset.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)