On Tue, Feb 10, 2015 at 05:46:13PM +0100, Alvaro Neira Ayuso wrote: > The function nft_jansson_parse_rule receives the tree like parameter and we > release it inside the functions. We use this function in the ruleset import > support. Therefore, we release the child nodes in the tree that contains the > rule information before to release the tree. Then, if we import a ruleset, > valgrind shows: > > ==16551== Invalid write of size 8 > ==16551== at 0x5EC92AA: json_delete (in > /usr/lib/x86_64-linux-gnu/libjansson.so.4.3.1) > ==16551== by 0x5EC4FC4: ??? (in > /usr/lib/x86_64-linux-gnu/libjansson.so.4.3.1) > ==16551== by 0x5EC5058: ??? (in > /usr/lib/x86_64-linux-gnu/libjansson.so.4.3.1) > ==16551== by 0x5EC9270: json_delete (in > /usr/lib/x86_64-linux-gnu/libjansson.so.4.3.1) > ==16551== by 0x5EC92B4: json_delete (in > /usr/lib/x86_64-linux-gnu/libjansson.so.4.3.1) > ==16551== by 0x5EC4FC4: ??? (in > /usr/lib/x86_64-linux-gnu/libjansson.so.4.3.1) > ==16551== by 0x5EC5058: ??? (in > /usr/lib/x86_64-linux-gnu/libjansson.so.4.3.1) > ==16551== by 0x5EC9270: json_delete (in > /usr/lib/x86_64-linux-gnu/libjansson.so.4.3.1) > ==16551== by 0x50455D0: nft_ruleset_json_parse (ruleset.c:557) > ==16551== by 0x50458AE: nft_ruleset_do_parse (ruleset.c:696) > ==16551== by 0x408AEC: do_command (rule.c:1317) > ==16551== by 0x406B05: nft_run (main.c:194) > > With this patch, we release the tree only in the function nft_rule_json_parse > (the caller function of nft_jansson_parse_rule). > > Signed-off-by: Alvaro Neira Ayuso <alvaroneay@xxxxxxxxx> > --- > src/rule.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/src/rule.c b/src/rule.c > index 7f4d049..028dc2e 100644 > --- a/src/rule.c > +++ b/src/rule.c > @@ -597,10 +597,8 @@ int nft_jansson_parse_rule(struct nft_rule *r, json_t *tree, > nft_rule_add_expr(r, e); > } > > - nft_jansson_free_root(tree); > return 0; > err: > - nft_jansson_free_root(tree); > return -1; > } > #endif > @@ -613,12 +611,16 @@ static int nft_rule_json_parse(struct nft_rule *r, const void *json, > #ifdef JSON_PARSING > json_t *tree; > json_error_t error; > + int ret; > > tree = nft_jansson_create_root(json, &error, err, input); > if (tree == NULL) > return -1; > > - return nft_jansson_parse_rule(r, tree, err, set_list); > + ret = nft_jansson_parse_rule(r, tree, err, set_list); > + > + nft_jansson_free_root(tree); I like this cleanup. It's a good idea to release things from the same function where you allocate for tracebility and readability reasons. But I don't understand why this is fixing the valgrind spot that you indicate above. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html