Re: [PATCH 4/4 v5] nftables: rule: Change the field "rule->comment" for an nftnl_udata_buf

["Carlos Falgueras García" <carlosfg@riseup.net>]

On 21/03/16 23:13, Pablo Neira Ayuso wrote:
> On Tue, Mar 15, 2016 at 09:28:07PM +0100, Carlos Falgueras García wrote:
>> Now it is possible to store multiple variable length user data into rule.
>> Modify the parser in order to fill the nftnl_udata with the comment, and
>> the print function for extract these commentary and print it to user.
>>
>> Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net>
>> ---
>>   include/rule.h            |  7 +++++++
>>   src/netlink_delinearize.c | 52 +++++++++++++++++++++++++++++++++++++++++++++--
>>   src/netlink_linearize.c   | 16 +++++++++++++--
>>   3 files changed, 71 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/rule.h b/include/rule.h
>> index c848f0f..b52f0ac 100644
>> --- a/include/rule.h
>> +++ b/include/rule.h
>> @@ -4,6 +4,7 @@
>>   #include <stdint.h>
>>   #include <nftables.h>
>>   #include <list.h>
>> +#include <libnftnl/udata.h>
>>
>>   /**
>>    * struct handle - handle for tables, chains, rules and sets
>> @@ -396,4 +397,10 @@ extern int do_command(struct netlink_ctx *ctx, struct cmd *cmd);
>>   extern int cache_update(enum cmd_ops cmd, struct list_head *msgs);
>>   extern void cache_release(void);
>>
>> +enum udata_type {
>> +	UDATA_TYPE_COMMENT,
>> +	__UDATA_TYPE_MAX,
>> +};
>> +#define UDATA_TYPE_MAX (__UDATA_TYPE_MAX - 1)
>> +
>>   #endif /* NFTABLES_RULE_H */
>> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
>> index d431588..5fcb5c1 100644
>> --- a/src/netlink_delinearize.c
>> +++ b/src/netlink_delinearize.c
>> @@ -25,6 +25,7 @@
>>   #include <utils.h>
>>   #include <erec.h>
>>   #include <sys/socket.h>
>> +#include <libnftnl/udata.h>
>>
>>   struct netlink_parse_ctx {
>>   	struct list_head	*msgs;
>> @@ -1746,6 +1747,54 @@ static void rule_parse_postprocess(struct netlink_parse_ctx *ctx, struct rule *r
>>   	}
>>   }
>>
>> +static int parse_udata_cb(const struct nftnl_udata *attr, void *data)
>> +{
>> +	unsigned char *value = nftnl_udata_attr_value(attr);
>> +	uint8_t type = nftnl_udata_attr_type(attr);
>> +	uint8_t len = nftnl_udata_attr_len(attr);
>> +	const struct nftnl_udata **tb = data;
>> +
>> +	switch (type) {
>> +	case UDATA_TYPE_COMMENT:
>> +		if (value[len - 1] != '\0')
>> +			return -1;
>> +		break;
>> +	default:
>> +		break;
>> +	};
>> +
>> +	tb[type] = attr;
>> +	return 1;
>> +}
>> +
>> +static char *udata_get_comment(const void *data, uint32_t data_len)
>> +{
>> +	const struct nftnl_udata *tb[UDATA_TYPE_MAX + 1] = {};
>> +	struct nftnl_udata_buf *udata;
>> +	uint8_t attr_len;
>> +	char *comment = NULL;
>> +
>> +	udata = nftnl_udata_alloc(data_len);
>> +	if (!udata)
>> +		memory_allocation_error();
>> +	nftnl_udata_copy_data(udata, data, data_len);
>> +
>> +	if (nftnl_udata_parse(udata, parse_udata_cb, tb) <= 0)
>> +		goto exit;
>
> I think this should be instead:
>
> 	if (nftnl_udata_parse(data, data_len, parse_udata_cb, tb) <= 0)
>
> So you don't need to allocate the buffer then copy data into it.
>
> I think the buffer infrastructure is only necessary to build the TLV
> attributes, not to parse it.
>

Ok.

>> +	if (!tb[UDATA_TYPE_COMMENT])
>> +		goto exit;
>> +
>> +	attr_len = nftnl_udata_attr_len(tb[UDATA_TYPE_COMMENT]);
>> +	comment = xmalloc(attr_len);
>> +	memcpy(comment, nftnl_udata_attr_value(tb[UDATA_TYPE_COMMENT]),
>> +	       attr_len);
>
> I'd suggest:
>
>          comment = xstrdup(nftnl_udata_attr_get_str(tb[UDATA_TYPE_COMMENT]));

I'll change this.

>> +
>> +exit:
>> +	nftnl_udata_free(udata);
>> +	return comment;
>> +}
>> +
>>   struct rule *netlink_delinearize_rule(struct netlink_ctx *ctx,
>>   				      const struct nftnl_rule *nlr)
>>   {
--
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