Re: [PATCH 2/4 v3] libnftnl: rule: Change the "userdata" attribute to use new TLV buffer

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Mon, Mar 07, 2016 at 06:10:42PM +0100, Carlos Falgueras García wrote:
> @@ -75,6 +81,8 @@ void nftnl_rule_free(struct nftnl_rule *r)
>  		xfree(r->table);
>  	if (r->chain != NULL)
>  		xfree(r->chain);
> +	if (r->flags & (1 << NFTNL_RULE_USERDATA))
> +		nftnl_udata_free(r->userdata);
>  
>  	xfree(r);
>  }
> @@ -162,8 +170,12 @@ void nftnl_rule_set_data(struct nftnl_rule *r, uint16_t attr,
>  		r->position = *((uint64_t *)data);
>  		break;
>  	case NFTNL_RULE_USERDATA:
> -		r->user.data = (void *)data;
> -		r->user.len = data_len;

You have to check here if r->userdata is already set, if so, release
it in first place.

> +		(r->userdata = nftnl_udata_alloc(data_len));
                ^                                         ^

You don't need these parens.

> +		if (!r->userdata) {
> +			perror("nftnl_rule_set_data - userdata");
> +			return;
> +		}
> +		nftnl_udata_copy_data(r->userdata, data, data_len);
>  		break;
>  	}
>  	r->flags |= (1 << attr);
> @@ -221,8 +233,8 @@ const void *nftnl_rule_get_data(const struct nftnl_rule *r, uint16_t attr,
>  		*data_len = sizeof(uint64_t);
>  		return &r->position;
>  	case NFTNL_RULE_USERDATA:
> -		*data_len = r->user.len;
> -		return r->user.data;
> +		*data_len = nftnl_udata_len(r->userdata);
> +		return (void *)nftnl_udata_data(r->userdata);
>  	}
>  	return NULL;
>  }
> @@ -288,8 +300,9 @@ void nftnl_rule_nlmsg_build_payload(struct nlmsghdr *nlh, struct nftnl_rule *r)
>  	if (r->flags & (1 << NFTNL_RULE_POSITION))
>  		mnl_attr_put_u64(nlh, NFTA_RULE_POSITION, htobe64(r->position));
>  	if (r->flags & (1 << NFTNL_RULE_USERDATA)) {
> -		mnl_attr_put(nlh, NFTA_RULE_USERDATA, r->user.len,
> -			     r->user.data);
> +		mnl_attr_put(nlh, NFTA_RULE_USERDATA,
> +			     nftnl_udata_len(r->userdata),
> +			     nftnl_udata_data(r->userdata));
>  	}
>  
>  	if (!list_empty(&r->expr_list)) {
> @@ -447,19 +460,17 @@ int nftnl_rule_nlmsg_parse(const struct nlmsghdr *nlh, struct nftnl_rule *r)
>  		r->flags |= (1 << NFTNL_RULE_POSITION);
>  	}
>  	if (tb[NFTA_RULE_USERDATA]) {
> +		uint16_t udata_size;

Missing line break after this.

>  		const void *udata =
>  			mnl_attr_get_payload(tb[NFTA_RULE_USERDATA]);
>  
> -		if (r->user.data)
> -			xfree(r->user.data);

These lines above are now gone, they avoid a memory leak.

> -
> -		r->user.len = mnl_attr_get_payload_len(tb[NFTA_RULE_USERDATA]);
> +		udata_size = mnl_attr_get_payload_len(tb[NFTA_RULE_USERDATA]);
>  
> -		r->user.data = malloc(r->user.len);
> -		if (r->user.data == NULL)
> +		(r->userdata = nftnl_udata_alloc(udata_size));
                ^                                           ^

No need for parens.

> +		if (!r->userdata)
>  			return -1;
> +		nftnl_udata_copy_data(r->userdata, udata, udata_size);
>  
> -		memcpy(r->user.data, udata, r->user.len);
>  		r->flags |= (1 << NFTNL_RULE_USERDATA);
>  	}
>  
> @@ -481,6 +492,7 @@ int nftnl_jansson_parse_rule(struct nftnl_rule *r, json_t *tree,
>  	uint64_t uval64;
>  	uint32_t uval32;
>  	int i, family;
> +	struct nftnl_udata_buf *buf;
>  
>  	root = nftnl_jansson_get_node(tree, "rule", err);
>  	if (root == NULL)
> @@ -557,6 +569,27 @@ int nftnl_jansson_parse_rule(struct nftnl_rule *r, json_t *tree,
>  		nftnl_rule_add_expr(r, e);
>  	}
>  
> +	array = json_object_get(root, "userdata");
> +	if (array != NULL) {
> +		buf = nftnl_udata_alloc(NFT_USERDATA_MAXLEN);
> +		if (!buf) {
> +			perror("nftnl_jansson_parse_rule");
> +			goto err;
> +		}
> +
> +		for (i = 0; i < json_array_size(array); ++i) {
> +			if (nftnl_jansson_udata_parse(buf,
> +						      json_array_get(array, i),
> +						      err,
> +						      set_list) < 0)
> +				goto err;
> +		}
> +
> +		nftnl_rule_set_data(r, NFTNL_RULE_USERDATA,
> +				    nftnl_udata_data(buf),
> +				    nftnl_udata_len(buf));
> +	}
> +
>  	return 0;
>  err:
>  	return -1;
> @@ -592,7 +625,7 @@ int nftnl_mxml_rule_parse(mxml_node_t *tree, struct nftnl_rule *r,
>  			struct nftnl_parse_err *err,
>  			struct nftnl_set_list *set_list)
>  {
> -	mxml_node_t *node;
> +	mxml_node_t *node, *node_ud;
>  	struct nftnl_expr *e;
>  	const char *table, *chain;
>  	int family;
> @@ -649,6 +682,35 @@ int nftnl_mxml_rule_parse(mxml_node_t *tree, struct nftnl_rule *r,
>  		nftnl_rule_add_expr(r, e);
>  	}
>  
> +	node_ud = mxmlFindElement(tree, tree, "userdata", NULL, NULL,
> +				  MXML_DESCEND);

You better wrap this code into a function in the mxml.c file as you
will need this later on for sets too. Same thing for the json code.
--
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