On 14/04/16 01:59, Pablo Neira Ayuso wrote:
On Tue, Mar 22, 2016 at 08:46:25PM +0100, Carlos Falgueras García wrote:
diff --git a/src/rule.c b/src/rule.c
index 3a32bf6..db96e5b 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -28,6 +28,7 @@
#include <libnftnl/rule.h>
#include <libnftnl/set.h>
#include <libnftnl/expr.h>
+#include <libnftnl/udata.h>
struct nftnl_rule {
struct list_head head;
@@ -38,10 +39,7 @@ struct nftnl_rule {
const char *chain;
uint64_t handle;
uint64_t position;
- struct {
- void *data;
- uint32_t len;
- } user;
+ struct nftnl_udata_buf *userdata;
This change, we don't need it, see below.
@@ -162,8 +171,14 @@ 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;
+ if (r->flags & (1 << NFTNL_RULE_USERDATA))
+ nftnl_udata_buf_free(r->userdata);
+ r->userdata = nftnl_udata_buf_alloc(data_len);
+ if (!r->userdata) {
+ perror("nftnl_rule_set_data - userdata");
+ return;
+ }
+ nftnl_udata_buf_put(r->userdata, data, data_len);
Think about this: From nft, we allocate the udata_buf, then we add the
attributes. Once we have the TLV area ready, we pass it as a pointer
via nftnl_rule_set_data().
Then, from libnftnl you allocate nftnl_udata_buf_alloc() again, but we
actually don't need this. The reason is that udata_buf is *only*
needed to build the sequence of TLVs. Once we're done with it, we can
just pass the memory blob area that contains this info.
I don't understand. With the modification to the patch [4/4 v6] you
made, there is still a data copy and it is passed in the same way. This
copy has moved from libnftnl to nft, but it still exists. Diff between
the original and modified version of "netlink_linearize_rule" function:
--- carlos/src/netlink_linearize.c
+++ pablo/src/netlink_linearize.c
@@ -1163,7 +1163,6 @@ void netlink_linearize_rule(struct netlink_ctx
*ctx, struct nftnl_rule *nlr,
const struct rule *rule)
{
struct netlink_linearize_ctx lctx;
- struct nftnl_udata_buf *udata;
const struct stmt *stmt;
memset(&lctx, 0, sizeof(lctx));
@@ -1174,6 +1173,10 @@ void netlink_linearize_rule(struct netlink_ctx
*ctx, struct nftnl_rule *nlr,
netlink_gen_stmt(&lctx, stmt);
if (rule->comment) {
+ struct nftnl_udata_buf *udata;
+ uint32_t udlen;
+ void *ud;
+
udata = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
if (!udata)
memory_allocation_error();
@@ -1182,9 +1185,11 @@ void netlink_linearize_rule(struct netlink_ctx
*ctx, struct nftnl_rule *nlr,
rule->comment))
memory_allocation_error();
- nftnl_rule_set_data(nlr, NFTNL_RULE_USERDATA,
- nftnl_udata_buf_data(udata),
- nftnl_udata_buf_len(udata));
+ udlen = nftnl_udata_buf_len(udata);
+ ud = xmalloc(udlen);
+ memcpy(ud, nftnl_udata_buf_data(udata), udlen);
+
+ nftnl_rule_set_data(nlr, NFTNL_RULE_USERDATA, ud, udlen);
nftnl_udata_buf_free(udata);
}
For this reason, I'm entirely keeping back this patch.
Good thing though is that we don't need it to get this working :-)
Now there are a couple of leaks because this patch added a couple of
neccessary 'frees':
@@ -75,6 +82,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_buf_free(r->userdata);
xfree(r);
}
and
@@ -162,8 +171,14 @@ 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;
+ if (r->flags & (1 << NFTNL_RULE_USERDATA))
+ nftnl_udata_buf_free(r->userdata);
+ r->userdata = nftnl_udata_buf_alloc(data_len);
+ if (!r->userdata) {
+ perror("nftnl_rule_set_data - userdata");
+ return;
+ }
+ nftnl_udata_buf_put(r->userdata, data, data_len);
break;
}
There are another 'frees' I think is better to change it for
'nftnl_udata_buf_free', although in practice it is the same.
If you agree, I'll send a modified version of patch [2/4 v6] without the
xml and json chunks, but keeping "struct nftnl_udata_buf" in "struct
nftnl_rule":
struct nftnl_rule {
struct list_head head;
@@ -38,10 +39,7 @@ struct nftnl_rule {
const char *chain;
uint64_t handle;
uint64_t position;
- struct {
- void *data;
- uint32_t len;
- } user;
+ struct nftnl_udata_buf *userdata;
struct {
uint32_t flags;
uint32_t proto;
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
