* [PATCH v2 nf-next] netfilter: nf_tables: add userdata attributes to nft_chain
@ 2020-09-23 10:55 Jose M. Guisado Gomez
2020-09-23 11:20 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Jose M. Guisado Gomez @ 2020-09-23 10:55 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
Enables storing userdata for nft_chain. Field udata points to user data
and udlen stores its length.
Adds new attribute flag NFTA_CHAIN_USERDATA.
Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
---
Fix error path and remove basic-like labels
include/net/netfilter/nf_tables.h | 2 ++
include/uapi/linux/netfilter/nf_tables.h | 2 ++
net/netfilter/nf_tables_api.c | 41 +++++++++++++++++++-----
3 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 8ceca0e419b3..4686fafbfd8a 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -952,6 +952,8 @@ struct nft_chain {
bound:1,
genmask:2;
char *name;
+ u16 udlen;
+ u8 *udata;
/* Only used during control plane commit phase: */
struct nft_rule **rules_next;
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 3c2469b43742..352ee51707a1 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -208,6 +208,7 @@ enum nft_chain_flags {
* @NFTA_CHAIN_COUNTERS: counter specification of the chain (NLA_NESTED: nft_counter_attributes)
* @NFTA_CHAIN_FLAGS: chain flags
* @NFTA_CHAIN_ID: uniquely identifies a chain in a transaction (NLA_U32)
+ * @NFTA_CHAIN_USERDATA: user data (NLA_BINARY)
*/
enum nft_chain_attributes {
NFTA_CHAIN_UNSPEC,
@@ -222,6 +223,7 @@ enum nft_chain_attributes {
NFTA_CHAIN_PAD,
NFTA_CHAIN_FLAGS,
NFTA_CHAIN_ID,
+ NFTA_CHAIN_USERDATA,
__NFTA_CHAIN_MAX
};
#define NFTA_CHAIN_MAX (__NFTA_CHAIN_MAX - 1)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 84c0c1aaae99..df800a20df59 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1306,6 +1306,8 @@ static const struct nla_policy nft_chain_policy[NFTA_CHAIN_MAX + 1] = {
[NFTA_CHAIN_COUNTERS] = { .type = NLA_NESTED },
[NFTA_CHAIN_FLAGS] = { .type = NLA_U32 },
[NFTA_CHAIN_ID] = { .type = NLA_U32 },
+ [NFTA_CHAIN_USERDATA] = { .type = NLA_BINARY,
+ .len = NFT_USERDATA_MAXLEN },
};
static const struct nla_policy nft_hook_policy[NFTA_HOOK_MAX + 1] = {
@@ -1447,6 +1449,10 @@ static int nf_tables_fill_chain_info(struct sk_buff *skb, struct net *net,
if (nla_put_be32(skb, NFTA_CHAIN_USE, htonl(chain->use)))
goto nla_put_failure;
+ if (chain->udata &&
+ nla_put(skb, NFTA_CHAIN_USERDATA, chain->udlen, chain->udata))
+ goto nla_put_failure;
+
nlmsg_end(skb, nlh);
return 0;
@@ -1978,6 +1984,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
struct nft_trans *trans;
struct nft_chain *chain;
struct nft_rule **rules;
+ u16 udlen = 0;
int err;
if (table->use == UINT_MAX)
@@ -2040,7 +2047,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
} else {
if (!(flags & NFT_CHAIN_BINDING)) {
err = -EINVAL;
- goto err1;
+ goto err_flag;
}
snprintf(name, sizeof(name), "__chain%llu", ++chain_id);
@@ -2049,13 +2056,25 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
if (!chain->name) {
err = -ENOMEM;
- goto err1;
+ goto err_chain_name;
+ }
+
+ if (nla[NFTA_CHAIN_USERDATA]) {
+ udlen = nla_len(nla[NFTA_CHAIN_USERDATA]);
+ chain->udata = kzalloc(udlen, GFP_KERNEL);
+ if (chain->udata == NULL) {
+ err = -ENOMEM;
+ goto err_userdata;
+ }
+
+ nla_memcpy(chain->udata, nla[NFTA_CHAIN_USERDATA], udlen);
+ chain->udlen = udlen;
}
rules = nf_tables_chain_alloc_rules(chain, 0);
if (!rules) {
err = -ENOMEM;
- goto err1;
+ goto err_alloc_rules;
}
*rules = NULL;
@@ -2064,12 +2083,12 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
err = nf_tables_register_hook(net, table, chain);
if (err < 0)
- goto err1;
+ goto err_register_hook;
trans = nft_trans_chain_add(ctx, NFT_MSG_NEWCHAIN);
if (IS_ERR(trans)) {
err = PTR_ERR(trans);
- goto err2;
+ goto err_trans;
}
nft_trans_chain_policy(trans) = NFT_CHAIN_POLICY_UNSET;
@@ -2079,15 +2098,21 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
err = nft_chain_add(table, chain);
if (err < 0) {
nft_trans_destroy(trans);
- goto err2;
+ goto err_chain_add;
}
table->use++;
return 0;
-err2:
+err_chain_add:
+err_trans:
nf_tables_unregister_hook(net, table, chain);
-err1:
+err_register_hook:
+err_alloc_rules:
+ kfree(chain->udata);
+err_userdata:
+err_chain_name:
+err_flag:
nf_tables_chain_destroy(ctx);
return err;
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 nf-next] netfilter: nf_tables: add userdata attributes to nft_chain
2020-09-23 10:55 [PATCH v2 nf-next] netfilter: nf_tables: add userdata attributes to nft_chain Jose M. Guisado Gomez
@ 2020-09-23 11:20 ` Pablo Neira Ayuso
2020-09-23 11:20 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2020-09-23 11:20 UTC (permalink / raw)
To: 20200921234912.GA6657; +Cc: netfilter-devel
On Wed, Sep 23, 2020 at 12:55:17PM +0200, Jose M. Guisado Gomez wrote:
> @@ -2079,15 +2098,21 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
> err = nft_chain_add(table, chain);
> if (err < 0) {
> nft_trans_destroy(trans);
> - goto err2;
> + goto err_chain_add;
> }
>
> table->use++;
>
> return 0;
> -err2:
> +err_chain_add:
> +err_trans:
I suggest to replace these two tags by:
err_unregister_hook:
> nf_tables_unregister_hook(net, table, chain);
> -err1:
> +err_register_hook:
> +err_alloc_rules:
and these two above by:
err_free_udata:
> + kfree(chain->udata);
> +err_userdata:
> +err_chain_name:
> +err_flag:
and these three by:
err_destroy_chain:
> nf_tables_chain_destroy(ctx);
>
> return err;
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2 nf-next] netfilter: nf_tables: add userdata attributes to nft_chain
2020-09-23 11:20 ` Pablo Neira Ayuso
@ 2020-09-23 11:20 ` Pablo Neira Ayuso
2020-09-23 13:16 ` [PATCH v3 " Jose M. Guisado Gomez
0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2020-09-23 11:20 UTC (permalink / raw)
To: Jose M. Guisado; +Cc: netfilter-devel
On Wed, Sep 23, 2020 at 12:55:17PM +0200, Jose M. Guisado Gomez wrote:
> @@ -2079,15 +2098,21 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
> err = nft_chain_add(table, chain);
> if (err < 0) {
> nft_trans_destroy(trans);
> - goto err2;
> + goto err_chain_add;
> }
>
> table->use++;
>
> return 0;
> -err2:
> +err_chain_add:
> +err_trans:
I suggest to replace these two tags by:
err_unregister_hook:
> nf_tables_unregister_hook(net, table, chain);
> -err1:
> +err_register_hook:
> +err_alloc_rules:
and these two above by:
err_free_udata:
> + kfree(chain->udata);
> +err_userdata:
> +err_chain_name:
> +err_flag:
and these three by:
err_destroy_chain:
> nf_tables_chain_destroy(ctx);
>
> return err;
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v3 nf-next] netfilter: nf_tables: add userdata attributes to nft_chain
2020-09-23 11:20 ` Pablo Neira Ayuso
@ 2020-09-23 13:16 ` Jose M. Guisado Gomez
2020-09-23 13:30 ` Florian Westphal
0 siblings, 1 reply; 5+ messages in thread
From: Jose M. Guisado Gomez @ 2020-09-23 13:16 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
Enables storing userdata for nft_chain. Field udata points to user data
and udlen stores its length.
Adds new attribute flag NFTA_CHAIN_USERDATA.
Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
---
Rename error path labels as proposed
include/net/netfilter/nf_tables.h | 2 ++
include/uapi/linux/netfilter/nf_tables.h | 2 ++
net/netfilter/nf_tables_api.c | 37 +++++++++++++++++++-----
3 files changed, 33 insertions(+), 8 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 8ceca0e419b3..4686fafbfd8a 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -952,6 +952,8 @@ struct nft_chain {
bound:1,
genmask:2;
char *name;
+ u16 udlen;
+ u8 *udata;
/* Only used during control plane commit phase: */
struct nft_rule **rules_next;
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 3c2469b43742..352ee51707a1 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -208,6 +208,7 @@ enum nft_chain_flags {
* @NFTA_CHAIN_COUNTERS: counter specification of the chain (NLA_NESTED: nft_counter_attributes)
* @NFTA_CHAIN_FLAGS: chain flags
* @NFTA_CHAIN_ID: uniquely identifies a chain in a transaction (NLA_U32)
+ * @NFTA_CHAIN_USERDATA: user data (NLA_BINARY)
*/
enum nft_chain_attributes {
NFTA_CHAIN_UNSPEC,
@@ -222,6 +223,7 @@ enum nft_chain_attributes {
NFTA_CHAIN_PAD,
NFTA_CHAIN_FLAGS,
NFTA_CHAIN_ID,
+ NFTA_CHAIN_USERDATA,
__NFTA_CHAIN_MAX
};
#define NFTA_CHAIN_MAX (__NFTA_CHAIN_MAX - 1)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 84c0c1aaae99..cde2c5194ed9 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1306,6 +1306,8 @@ static const struct nla_policy nft_chain_policy[NFTA_CHAIN_MAX + 1] = {
[NFTA_CHAIN_COUNTERS] = { .type = NLA_NESTED },
[NFTA_CHAIN_FLAGS] = { .type = NLA_U32 },
[NFTA_CHAIN_ID] = { .type = NLA_U32 },
+ [NFTA_CHAIN_USERDATA] = { .type = NLA_BINARY,
+ .len = NFT_USERDATA_MAXLEN },
};
static const struct nla_policy nft_hook_policy[NFTA_HOOK_MAX + 1] = {
@@ -1447,6 +1449,10 @@ static int nf_tables_fill_chain_info(struct sk_buff *skb, struct net *net,
if (nla_put_be32(skb, NFTA_CHAIN_USE, htonl(chain->use)))
goto nla_put_failure;
+ if (chain->udata &&
+ nla_put(skb, NFTA_CHAIN_USERDATA, chain->udlen, chain->udata))
+ goto nla_put_failure;
+
nlmsg_end(skb, nlh);
return 0;
@@ -1978,6 +1984,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
struct nft_trans *trans;
struct nft_chain *chain;
struct nft_rule **rules;
+ u16 udlen = 0;
int err;
if (table->use == UINT_MAX)
@@ -2040,7 +2047,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
} else {
if (!(flags & NFT_CHAIN_BINDING)) {
err = -EINVAL;
- goto err1;
+ goto err_destroy_chain;
}
snprintf(name, sizeof(name), "__chain%llu", ++chain_id);
@@ -2049,13 +2056,25 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
if (!chain->name) {
err = -ENOMEM;
- goto err1;
+ goto err_destroy_chain;
+ }
+
+ if (nla[NFTA_CHAIN_USERDATA]) {
+ udlen = nla_len(nla[NFTA_CHAIN_USERDATA]);
+ chain->udata = kzalloc(udlen, GFP_KERNEL);
+ if (chain->udata == NULL) {
+ err = -ENOMEM;
+ goto err_destroy_chain;
+ }
+
+ nla_memcpy(chain->udata, nla[NFTA_CHAIN_USERDATA], udlen);
+ chain->udlen = udlen;
}
rules = nf_tables_chain_alloc_rules(chain, 0);
if (!rules) {
err = -ENOMEM;
- goto err1;
+ goto err_free_udata;
}
*rules = NULL;
@@ -2064,12 +2083,12 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
err = nf_tables_register_hook(net, table, chain);
if (err < 0)
- goto err1;
+ goto err_free_udata;
trans = nft_trans_chain_add(ctx, NFT_MSG_NEWCHAIN);
if (IS_ERR(trans)) {
err = PTR_ERR(trans);
- goto err2;
+ goto err_unregister_hook;
}
nft_trans_chain_policy(trans) = NFT_CHAIN_POLICY_UNSET;
@@ -2079,15 +2098,17 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
err = nft_chain_add(table, chain);
if (err < 0) {
nft_trans_destroy(trans);
- goto err2;
+ goto err_unregister_hook;
}
table->use++;
return 0;
-err2:
+err_unregister_hook:
nf_tables_unregister_hook(net, table, chain);
-err1:
+err_free_udata:
+ kfree(chain->udata);
+err_destroy_chain:
nf_tables_chain_destroy(ctx);
return err;
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v3 nf-next] netfilter: nf_tables: add userdata attributes to nft_chain
2020-09-23 13:16 ` [PATCH v3 " Jose M. Guisado Gomez
@ 2020-09-23 13:30 ` Florian Westphal
0 siblings, 0 replies; 5+ messages in thread
From: Florian Westphal @ 2020-09-23 13:30 UTC (permalink / raw)
To: Jose M. Guisado Gomez; +Cc: pablo, netfilter-devel
Jose M. Guisado Gomez <guigom@riseup.net> wrote:
> Enables storing userdata for nft_chain. Field udata points to user data
> and udlen stores its length.
>
> Adds new attribute flag NFTA_CHAIN_USERDATA.
>
> Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
> ---
> + if (nla[NFTA_CHAIN_USERDATA]) {
> + udlen = nla_len(nla[NFTA_CHAIN_USERDATA]);
> + chain->udata = kzalloc(udlen, GFP_KERNEL);
> + if (chain->udata == NULL) {
> + err = -ENOMEM;
> + goto err_destroy_chain;
> + }
> +
> + nla_memcpy(chain->udata, nla[NFTA_CHAIN_USERDATA], udlen);
> + chain->udlen = udlen;
nit: You could use nla_memdup() instead of alloc+memcpy.
> -err2:
> +err_unregister_hook:
> nf_tables_unregister_hook(net, table, chain);
> -err1:
> +err_free_udata:
> + kfree(chain->udata);
> +err_destroy_chain:
> nf_tables_chain_destroy(ctx);
This frees ->udata on error. But what if the chain is added
successfully and then deleted at a later time?
Wouldn't it make more sense to only patch nf_tables_chain_destroy()
to handle both error and chain delete case?
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-23 13:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-23 10:55 [PATCH v2 nf-next] netfilter: nf_tables: add userdata attributes to nft_chain Jose M. Guisado Gomez
2020-09-23 11:20 ` Pablo Neira Ayuso
2020-09-23 11:20 ` Pablo Neira Ayuso
2020-09-23 13:16 ` [PATCH v3 " Jose M. Guisado Gomez
2020-09-23 13:30 ` Florian Westphal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).