* [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions
@ 2017-07-27 14:56 Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 1/6] netfilter: nf_tables: No need to check chain existence when tracing Phil Sutter
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: Phil Sutter @ 2017-07-27 14:56 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
The following series lifts the tight restriction on name length of
tables, chains, sets and objects. This is done by allocating memory for
names dynamically, so there is no added overhead when reducing the
restriction to a mere sanity level of 255 characters.
The first patch removes a needless check discovered when discussing v2
of this patch set.
The second patch introduces nla_strdup() which aids in duplicating a
string contained in a netlink attribute. It is used to replace the call
to nla_strlcpy() when populating name fields.
I've tested the series manually by creating tables, chains, sets and
counter objects with long names and automated by running the py and
shell testsuites of nftables repo. Also, kmemleak did not find anything
nftables related.
Changes since v2:
- Added new patch 1.
- Patch 2 remains unchanged.
- Detailed changelog of remaining patches is found there.
Phil Sutter (6):
netfilter: nf_tables: No need to check chain existence when tracing
networking: Introduce nla_strdup()
netfilter: nf_tables: Allow table names of up to 255 chars
netfilter: nf_tables: Allow chain name of up to 255 chars
netfilter: nf_tables: Allow set names of up to 255 chars
netfilter: nf_tables: Allow object names of up to 255 chars
include/net/netfilter/nf_tables.h | 10 +--
include/net/netlink.h | 1 +
include/uapi/linux/netfilter/nf_tables.h | 9 +--
lib/nlattr.c | 24 +++++++
net/netfilter/nf_tables_api.c | 112 +++++++++++++++++++++++--------
net/netfilter/nf_tables_trace.c | 42 +++++++++---
6 files changed, 151 insertions(+), 47 deletions(-)
--
2.13.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [nf-next PATCH v3 1/6] netfilter: nf_tables: No need to check chain existence when tracing
2017-07-27 14:56 [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions Phil Sutter
@ 2017-07-27 14:56 ` Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 2/6] networking: Introduce nla_strdup() Phil Sutter
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Phil Sutter @ 2017-07-27 14:56 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
nft_trace_notify() is called only from __nft_trace_packet(), which
assigns its parameter 'chain' to info->chain. __nft_trace_packet() in
turn later dereferences 'chain' unconditionally, which indicates that
it's never NULL. Same does nft_do_chain(), the only user of the tracing
infrastructure. Hence it is safe to assume the check removed here is not
needed.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
net/netfilter/nf_tables_trace.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/net/netfilter/nf_tables_trace.c b/net/netfilter/nf_tables_trace.c
index e1b15e7a5793f..0c3a0049e4aa2 100644
--- a/net/netfilter/nf_tables_trace.c
+++ b/net/netfilter/nf_tables_trace.c
@@ -217,14 +217,11 @@ void nft_trace_notify(struct nft_traceinfo *info)
if (trace_fill_id(skb, pkt->skb))
goto nla_put_failure;
- if (info->chain) {
- if (nla_put_string(skb, NFTA_TRACE_CHAIN,
- info->chain->name))
- goto nla_put_failure;
- if (nla_put_string(skb, NFTA_TRACE_TABLE,
- info->chain->table->name))
- goto nla_put_failure;
- }
+ if (nla_put_string(skb, NFTA_TRACE_CHAIN, info->chain->name))
+ goto nla_put_failure;
+
+ if (nla_put_string(skb, NFTA_TRACE_TABLE, info->chain->table->name))
+ goto nla_put_failure;
if (nf_trace_fill_rule_info(skb, info))
goto nla_put_failure;
--
2.13.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [nf-next PATCH v3 2/6] networking: Introduce nla_strdup()
2017-07-27 14:56 [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 1/6] netfilter: nf_tables: No need to check chain existence when tracing Phil Sutter
@ 2017-07-27 14:56 ` Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 3/6] netfilter: nf_tables: Allow table names of up to 255 chars Phil Sutter
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Phil Sutter @ 2017-07-27 14:56 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
This is similar to strdup() for netlink string attributes.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
include/net/netlink.h | 1 +
lib/nlattr.c | 24 ++++++++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index ef8e6c3a80a63..c8c2eb5ae55ef 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -247,6 +247,7 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
int nla_policy_len(const struct nla_policy *, int);
struct nlattr *nla_find(const struct nlattr *head, int len, int attrtype);
size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize);
+char *nla_strdup(const struct nlattr *nla, gfp_t flags);
int nla_memcpy(void *dest, const struct nlattr *src, int count);
int nla_memcmp(const struct nlattr *nla, const void *data, size_t size);
int nla_strcmp(const struct nlattr *nla, const char *str);
diff --git a/lib/nlattr.c b/lib/nlattr.c
index fb52435be42dd..f13013f7e21a1 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -272,6 +272,30 @@ size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize)
EXPORT_SYMBOL(nla_strlcpy);
/**
+ * nla_strdup - Copy string attribute payload into a newly allocated buffer
+ * @nla: attribute to copy the string from
+ * @flags: the type of memory to allocate (see kmalloc).
+ *
+ * Returns a pointer to the allocated buffer or NULL on error.
+ */
+char *nla_strdup(const struct nlattr *nla, gfp_t flags)
+{
+ size_t srclen = nla_len(nla);
+ char *src = nla_data(nla), *dst;
+
+ if (srclen > 0 && src[srclen - 1] == '\0')
+ srclen--;
+
+ dst = kmalloc(srclen + 1, flags);
+ if (dst != NULL) {
+ memcpy(dst, src, srclen);
+ dst[srclen] = '\0';
+ }
+ return dst;
+}
+EXPORT_SYMBOL(nla_strdup);
+
+/**
* nla_memcpy - Copy a netlink attribute into another memory area
* @dest: where to copy to memcpy
* @src: netlink attribute to copy from
--
2.13.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [nf-next PATCH v3 3/6] netfilter: nf_tables: Allow table names of up to 255 chars
2017-07-27 14:56 [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 1/6] netfilter: nf_tables: No need to check chain existence when tracing Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 2/6] networking: Introduce nla_strdup() Phil Sutter
@ 2017-07-27 14:56 ` Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 4/6] netfilter: nf_tables: Allow chain name " Phil Sutter
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Phil Sutter @ 2017-07-27 14:56 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Allocate all table names dynamically to allow for arbitrary lengths but
introduce NFT_NAME_MAXLEN as an upper sanity boundary. It's value was
chosen to allow using a domain name as per RFC 1035.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Introduce NFT_NAME_MAXLEN as an upper boundary to restrict overly long
names but still allow to use e.g. domain names.
- Adjust commit messages accordingly.
Changes since v2:
- Adjust commit summary - length is not really unlimited.
- Don't drop NFT_TABLE_MAXNAMELEN macro since it's UAPI.
- Keep using NFT_TABLE_MAXNAMELEN in netlink policies to reduce patch
size.
- Add size of chain's table name unconditionally to nlmsg size in
nft_trace_notify() since info->chain should always be present.
---
include/net/netfilter/nf_tables.h | 2 +-
include/uapi/linux/netfilter/nf_tables.h | 3 +-
net/netfilter/nf_tables_api.c | 49 +++++++++++++++++++++++---------
net/netfilter/nf_tables_trace.c | 2 +-
4 files changed, 40 insertions(+), 16 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index bd5be0d691d51..05ecf78ec0787 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -957,7 +957,7 @@ struct nft_table {
u32 use;
u16 flags:14,
genmask:2;
- char name[NFT_TABLE_MAXNAMELEN];
+ char *name;
};
enum nft_af_flags {
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 683f6f88fcace..f796da401672b 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -1,7 +1,8 @@
#ifndef _LINUX_NF_TABLES_H
#define _LINUX_NF_TABLES_H
-#define NFT_TABLE_MAXNAMELEN 32
+#define NFT_NAME_MAXLEN 256
+#define NFT_TABLE_MAXNAMELEN NFT_NAME_MAXLEN
#define NFT_CHAIN_MAXNAMELEN 32
#define NFT_SET_MAXNAMELEN 32
#define NFT_OBJ_MAXNAMELEN 32
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 7843efa33c598..dd824def0a182 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -726,7 +726,10 @@ static int nf_tables_newtable(struct net *net, struct sock *nlsk,
if (table == NULL)
goto err2;
- nla_strlcpy(table->name, name, NFT_TABLE_MAXNAMELEN);
+ table->name = nla_strdup(name, GFP_KERNEL);
+ if (table->name == NULL)
+ goto err3;
+
INIT_LIST_HEAD(&table->chains);
INIT_LIST_HEAD(&table->sets);
INIT_LIST_HEAD(&table->objects);
@@ -735,10 +738,12 @@ static int nf_tables_newtable(struct net *net, struct sock *nlsk,
nft_ctx_init(&ctx, net, skb, nlh, afi, table, NULL, nla);
err = nft_trans_table_add(&ctx, NFT_MSG_NEWTABLE);
if (err < 0)
- goto err3;
+ goto err4;
list_add_tail_rcu(&table->list, &afi->tables);
return 0;
+err4:
+ kfree(table->name);
err3:
kfree(table);
err2:
@@ -865,6 +870,7 @@ static void nf_tables_table_destroy(struct nft_ctx *ctx)
{
BUG_ON(ctx->table->use > 0);
+ kfree(ctx->table->name);
kfree(ctx->table);
module_put(ctx->afi->owner);
}
@@ -1977,7 +1983,7 @@ static void nf_tables_rule_notify(const struct nft_ctx *ctx,
}
struct nft_rule_dump_ctx {
- char table[NFT_TABLE_MAXNAMELEN];
+ char *table;
char chain[NFT_CHAIN_MAXNAMELEN];
};
@@ -2002,7 +2008,7 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
continue;
list_for_each_entry_rcu(table, &afi->tables, list) {
- if (ctx && ctx->table[0] &&
+ if (ctx && ctx->table &&
strcmp(ctx->table, table->name) != 0)
continue;
@@ -2042,7 +2048,12 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
static int nf_tables_dump_rules_done(struct netlink_callback *cb)
{
- kfree(cb->data);
+ struct nft_rule_dump_ctx *ctx = cb->data;
+
+ if (ctx) {
+ kfree(ctx->table);
+ kfree(ctx);
+ }
return 0;
}
@@ -2074,9 +2085,14 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk,
if (!ctx)
return -ENOMEM;
- if (nla[NFTA_RULE_TABLE])
- nla_strlcpy(ctx->table, nla[NFTA_RULE_TABLE],
- sizeof(ctx->table));
+ if (nla[NFTA_RULE_TABLE]) {
+ ctx->table = nla_strdup(nla[NFTA_RULE_TABLE],
+ GFP_KERNEL);
+ if (!ctx->table) {
+ kfree(ctx);
+ return -ENOMEM;
+ }
+ }
if (nla[NFTA_RULE_CHAIN])
nla_strlcpy(ctx->chain, nla[NFTA_RULE_CHAIN],
sizeof(ctx->chain));
@@ -4415,7 +4431,7 @@ static int nf_tables_fill_obj_info(struct sk_buff *skb, struct net *net,
}
struct nft_obj_filter {
- char table[NFT_OBJ_MAXNAMELEN];
+ char *table;
u32 type;
};
@@ -4480,7 +4496,10 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
static int nf_tables_dump_obj_done(struct netlink_callback *cb)
{
- kfree(cb->data);
+ struct nft_obj_filter *filter = cb->data;
+
+ kfree(filter->table);
+ kfree(filter);
return 0;
}
@@ -4494,9 +4513,13 @@ nft_obj_filter_alloc(const struct nlattr * const nla[])
if (!filter)
return ERR_PTR(-ENOMEM);
- if (nla[NFTA_OBJ_TABLE])
- nla_strlcpy(filter->table, nla[NFTA_OBJ_TABLE],
- NFT_TABLE_MAXNAMELEN);
+ if (nla[NFTA_OBJ_TABLE]) {
+ filter->table = nla_strdup(nla[NFTA_OBJ_TABLE], GFP_KERNEL);
+ if (!filter->table) {
+ kfree(filter);
+ return ERR_PTR(-ENOMEM);
+ }
+ }
if (nla[NFTA_OBJ_TYPE])
filter->type = ntohl(nla_get_be32(nla[NFTA_OBJ_TYPE]));
diff --git a/net/netfilter/nf_tables_trace.c b/net/netfilter/nf_tables_trace.c
index 0c3a0049e4aa2..62787d985e9d3 100644
--- a/net/netfilter/nf_tables_trace.c
+++ b/net/netfilter/nf_tables_trace.c
@@ -175,7 +175,7 @@ void nft_trace_notify(struct nft_traceinfo *info)
return;
size = nlmsg_total_size(sizeof(struct nfgenmsg)) +
- nla_total_size(NFT_TABLE_MAXNAMELEN) +
+ nla_total_size(strlen(info->chain->table->name)) +
nla_total_size(NFT_CHAIN_MAXNAMELEN) +
nla_total_size_64bit(sizeof(__be64)) + /* rule handle */
nla_total_size(sizeof(__be32)) + /* trace type */
--
2.13.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [nf-next PATCH v3 4/6] netfilter: nf_tables: Allow chain name of up to 255 chars
2017-07-27 14:56 [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions Phil Sutter
` (2 preceding siblings ...)
2017-07-27 14:56 ` [nf-next PATCH v3 3/6] netfilter: nf_tables: Allow table names of up to 255 chars Phil Sutter
@ 2017-07-27 14:56 ` Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 5/6] netfilter: nf_tables: Allow set names " Phil Sutter
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Phil Sutter @ 2017-07-27 14:56 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Same conversion as for table names, use NFT_NAME_MAXLEN as upper
boundary as well.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Introduce NFT_NAME_MAXLEN as an upper boundary to restrict overly long
names but still allow to use e.g. domain names.
- Adjust commit messages accordingly.
Changes since v2:
- Adjust commit summary - length is not really unlimited.
- Don't drop NFT_CHAIN_MAXNAMELEN macro since it's UAPI.
- Keep using NFT_CHAIN_MAXNAMELEN in netlink policies to reduce patch
size.
- Be more pedantic when checking for existence of
info->verdict->chain->name.
---
include/net/netfilter/nf_tables.h | 4 ++--
include/uapi/linux/netfilter/nf_tables.h | 2 +-
net/netfilter/nf_tables_api.c | 34 ++++++++++++++++++++++++--------
net/netfilter/nf_tables_trace.c | 27 +++++++++++++++++++++++--
4 files changed, 54 insertions(+), 13 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 05ecf78ec0787..be1610162ee02 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -859,7 +859,7 @@ struct nft_chain {
u16 level;
u8 flags:6,
genmask:2;
- char name[NFT_CHAIN_MAXNAMELEN];
+ char *name;
};
enum nft_chain_type {
@@ -1272,7 +1272,7 @@ struct nft_trans_set {
struct nft_trans_chain {
bool update;
- char name[NFT_CHAIN_MAXNAMELEN];
+ char *name;
struct nft_stats __percpu *stats;
u8 policy;
};
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index f796da401672b..6b586f43bd53a 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -3,7 +3,7 @@
#define NFT_NAME_MAXLEN 256
#define NFT_TABLE_MAXNAMELEN NFT_NAME_MAXLEN
-#define NFT_CHAIN_MAXNAMELEN 32
+#define NFT_CHAIN_MAXNAMELEN NFT_NAME_MAXLEN
#define NFT_SET_MAXNAMELEN 32
#define NFT_OBJ_MAXNAMELEN 32
#define NFT_USERDATA_MAXLEN 256
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index dd824def0a182..55368ad7b9e7a 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1248,8 +1248,10 @@ static void nf_tables_chain_destroy(struct nft_chain *chain)
free_percpu(basechain->stats);
if (basechain->ops[0].dev != NULL)
dev_put(basechain->ops[0].dev);
+ kfree(chain->name);
kfree(basechain);
} else {
+ kfree(chain->name);
kfree(chain);
}
}
@@ -1474,8 +1476,13 @@ static int nf_tables_newchain(struct net *net, struct sock *nlsk,
nft_trans_chain_policy(trans) = -1;
if (nla[NFTA_CHAIN_HANDLE] && name) {
- nla_strlcpy(nft_trans_chain_name(trans), name,
- NFT_CHAIN_MAXNAMELEN);
+ nft_trans_chain_name(trans) =
+ nla_strdup(name, GFP_KERNEL);
+ if (!nft_trans_chain_name(trans)) {
+ kfree(trans);
+ free_percpu(stats);
+ return -ENOMEM;
+ }
}
list_add_tail(&trans->list, &net->nft.commit_list);
return 0;
@@ -1549,7 +1556,11 @@ static int nf_tables_newchain(struct net *net, struct sock *nlsk,
INIT_LIST_HEAD(&chain->rules);
chain->handle = nf_tables_alloc_handle(table);
chain->table = table;
- nla_strlcpy(chain->name, name, NFT_CHAIN_MAXNAMELEN);
+ chain->name = nla_strdup(name, GFP_KERNEL);
+ if (!chain->name) {
+ err = -ENOMEM;
+ goto err1;
+ }
err = nf_tables_register_hooks(net, table, chain, afi->nops);
if (err < 0)
@@ -1984,7 +1995,7 @@ static void nf_tables_rule_notify(const struct nft_ctx *ctx,
struct nft_rule_dump_ctx {
char *table;
- char chain[NFT_CHAIN_MAXNAMELEN];
+ char *chain;
};
static int nf_tables_dump_rules(struct sk_buff *skb,
@@ -2052,6 +2063,7 @@ static int nf_tables_dump_rules_done(struct netlink_callback *cb)
if (ctx) {
kfree(ctx->table);
+ kfree(ctx->chain);
kfree(ctx);
}
return 0;
@@ -2093,9 +2105,15 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk,
return -ENOMEM;
}
}
- if (nla[NFTA_RULE_CHAIN])
- nla_strlcpy(ctx->chain, nla[NFTA_RULE_CHAIN],
- sizeof(ctx->chain));
+ if (nla[NFTA_RULE_CHAIN]) {
+ ctx->chain = nla_strdup(nla[NFTA_RULE_CHAIN],
+ GFP_KERNEL);
+ if (!ctx->chain) {
+ kfree(ctx->table);
+ kfree(ctx);
+ return -ENOMEM;
+ }
+ }
c.data = ctx;
}
@@ -4865,7 +4883,7 @@ static void nft_chain_commit_update(struct nft_trans *trans)
{
struct nft_base_chain *basechain;
- if (nft_trans_chain_name(trans)[0])
+ if (nft_trans_chain_name(trans))
strcpy(trans->ctx.chain->name, nft_trans_chain_name(trans));
if (!nft_is_base_chain(trans->ctx.chain))
diff --git a/net/netfilter/nf_tables_trace.c b/net/netfilter/nf_tables_trace.c
index 62787d985e9d3..e1dc527a493b8 100644
--- a/net/netfilter/nf_tables_trace.c
+++ b/net/netfilter/nf_tables_trace.c
@@ -162,6 +162,27 @@ static int nf_trace_fill_rule_info(struct sk_buff *nlskb,
NFTA_TRACE_PAD);
}
+static bool nft_trace_have_verdict_chain(struct nft_traceinfo *info)
+{
+ switch (info->type) {
+ case NFT_TRACETYPE_RETURN:
+ case NFT_TRACETYPE_RULE:
+ break;
+ default:
+ return false;
+ }
+
+ switch (info->verdict->code) {
+ case NFT_JUMP:
+ case NFT_GOTO:
+ break;
+ default:
+ return false;
+ }
+
+ return true;
+}
+
void nft_trace_notify(struct nft_traceinfo *info)
{
const struct nft_pktinfo *pkt = info->pkt;
@@ -176,12 +197,11 @@ void nft_trace_notify(struct nft_traceinfo *info)
size = nlmsg_total_size(sizeof(struct nfgenmsg)) +
nla_total_size(strlen(info->chain->table->name)) +
- nla_total_size(NFT_CHAIN_MAXNAMELEN) +
+ nla_total_size(strlen(info->chain->name)) +
nla_total_size_64bit(sizeof(__be64)) + /* rule handle */
nla_total_size(sizeof(__be32)) + /* trace type */
nla_total_size(0) + /* VERDICT, nested */
nla_total_size(sizeof(u32)) + /* verdict code */
- nla_total_size(NFT_CHAIN_MAXNAMELEN) + /* jump target */
nla_total_size(sizeof(u32)) + /* id */
nla_total_size(NFT_TRACETYPE_LL_HSIZE) +
nla_total_size(NFT_TRACETYPE_NETWORK_HSIZE) +
@@ -194,6 +214,9 @@ void nft_trace_notify(struct nft_traceinfo *info)
nla_total_size(sizeof(u32)) + /* nfproto */
nla_total_size(sizeof(u32)); /* policy */
+ if (nft_trace_have_verdict_chain(info))
+ size += nla_total_size(strlen(info->verdict->chain->name)); /* jump target */
+
skb = nlmsg_new(size, GFP_ATOMIC);
if (!skb)
return;
--
2.13.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [nf-next PATCH v3 5/6] netfilter: nf_tables: Allow set names of up to 255 chars
2017-07-27 14:56 [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions Phil Sutter
` (3 preceding siblings ...)
2017-07-27 14:56 ` [nf-next PATCH v3 4/6] netfilter: nf_tables: Allow chain name " Phil Sutter
@ 2017-07-27 14:56 ` Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 6/6] netfilter: nf_tables: Allow object " Phil Sutter
2017-07-31 17:17 ` [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions Pablo Neira Ayuso
6 siblings, 0 replies; 8+ messages in thread
From: Phil Sutter @ 2017-07-27 14:56 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Same conversion as for table names, use NFT_NAME_MAXLEN as upper
boundary as well.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Introduce NFT_NAME_MAXLEN as an upper boundary to restrict overly long
names but still allow to use e.g. domain names.
- Adjust commit messages accordingly.
Changes since v2:
- Adjust commit summary - length is not really unlimited.
- Don't drop NFT_SET_MAXNAMELEN macro since it's UAPI.
- Keep using NFT_SET_MAXNAMELEN in netlink policies to reduce patch
size.
---
include/net/netfilter/nf_tables.h | 2 +-
include/uapi/linux/netfilter/nf_tables.h | 2 +-
net/netfilter/nf_tables_api.c | 18 ++++++++++++++----
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index be1610162ee02..66ba62fa7d90e 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -396,7 +396,7 @@ void nft_unregister_set(struct nft_set_type *type);
struct nft_set {
struct list_head list;
struct list_head bindings;
- char name[NFT_SET_MAXNAMELEN];
+ char *name;
u32 ktype;
u32 dtype;
u32 objtype;
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 6b586f43bd53a..375528546a968 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -4,7 +4,7 @@
#define NFT_NAME_MAXLEN 256
#define NFT_TABLE_MAXNAMELEN NFT_NAME_MAXLEN
#define NFT_CHAIN_MAXNAMELEN NFT_NAME_MAXLEN
-#define NFT_SET_MAXNAMELEN 32
+#define NFT_SET_MAXNAMELEN NFT_NAME_MAXLEN
#define NFT_OBJ_MAXNAMELEN 32
#define NFT_USERDATA_MAXLEN 256
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 55368ad7b9e7a..72672b557d5c8 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2655,7 +2655,7 @@ static int nf_tables_set_alloc_name(struct nft_ctx *ctx, struct nft_set *set,
unsigned long *inuse;
unsigned int n = 0, min = 0;
- p = strnchr(name, NFT_SET_MAXNAMELEN, '%');
+ p = strchr(name, '%');
if (p != NULL) {
if (p[1] != 'd' || strchr(p + 2, '%'))
return -EINVAL;
@@ -2686,7 +2686,10 @@ static int nf_tables_set_alloc_name(struct nft_ctx *ctx, struct nft_set *set,
free_page((unsigned long)inuse);
}
- snprintf(set->name, sizeof(set->name), name, min + n);
+ set->name = kasprintf(GFP_KERNEL, name, min + n);
+ if (!set->name)
+ return -ENOMEM;
+
list_for_each_entry(i, &ctx->table->sets, list) {
if (!nft_is_active_next(ctx->net, i))
continue;
@@ -2963,7 +2966,7 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk,
struct nft_table *table;
struct nft_set *set;
struct nft_ctx ctx;
- char name[NFT_SET_MAXNAMELEN];
+ char *name;
unsigned int size;
bool create;
u64 timeout;
@@ -3109,8 +3112,14 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk,
goto err1;
}
- nla_strlcpy(name, nla[NFTA_SET_NAME], sizeof(set->name));
+ name = nla_strdup(nla[NFTA_SET_NAME], GFP_KERNEL);
+ if (!name) {
+ err = -ENOMEM;
+ goto err2;
+ }
+
err = nf_tables_set_alloc_name(&ctx, set, name);
+ kfree(name);
if (err < 0)
goto err2;
@@ -3160,6 +3169,7 @@ static void nft_set_destroy(struct nft_set *set)
{
set->ops->destroy(set);
module_put(set->ops->type->owner);
+ kfree(set->name);
kvfree(set);
}
--
2.13.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [nf-next PATCH v3 6/6] netfilter: nf_tables: Allow object names of up to 255 chars
2017-07-27 14:56 [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions Phil Sutter
` (4 preceding siblings ...)
2017-07-27 14:56 ` [nf-next PATCH v3 5/6] netfilter: nf_tables: Allow set names " Phil Sutter
@ 2017-07-27 14:56 ` Phil Sutter
2017-07-31 17:17 ` [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions Pablo Neira Ayuso
6 siblings, 0 replies; 8+ messages in thread
From: Phil Sutter @ 2017-07-27 14:56 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Same conversion as for table names, use NFT_NAME_MAXLEN as upper
boundary as well.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Introduce NFT_NAME_MAXLEN as an upper boundary to restrict overly long
names but still allow to use e.g. domain names.
- Adjust commit messages accordingly.
Changes since v2:
- Adjust commit summary - length is not really unlimited.
- Don't drop NFT_OBJ_MAXNAMELEN macro since it's UAPI.
- Keep using NFT_OBJ_MAXNAMELEN in netlink policies to reduce patch
size.
---
include/net/netfilter/nf_tables.h | 2 +-
include/uapi/linux/netfilter/nf_tables.h | 2 +-
net/netfilter/nf_tables_api.c | 11 +++++++++--
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 66ba62fa7d90e..f9795fe394f31 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1016,7 +1016,7 @@ int nft_verdict_dump(struct sk_buff *skb, int type,
*/
struct nft_object {
struct list_head list;
- char name[NFT_OBJ_MAXNAMELEN];
+ char *name;
struct nft_table *table;
u32 genmask:2,
use:30;
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 375528546a968..d72fda5f01b86 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -5,7 +5,7 @@
#define NFT_TABLE_MAXNAMELEN NFT_NAME_MAXLEN
#define NFT_CHAIN_MAXNAMELEN NFT_NAME_MAXLEN
#define NFT_SET_MAXNAMELEN NFT_NAME_MAXLEN
-#define NFT_OBJ_MAXNAMELEN 32
+#define NFT_OBJ_MAXNAMELEN NFT_NAME_MAXLEN
#define NFT_USERDATA_MAXLEN 256
/**
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 72672b557d5c8..beb17b462da34 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -4407,15 +4407,21 @@ static int nf_tables_newobj(struct net *net, struct sock *nlsk,
goto err1;
}
obj->table = table;
- nla_strlcpy(obj->name, nla[NFTA_OBJ_NAME], NFT_OBJ_MAXNAMELEN);
+ obj->name = nla_strdup(nla[NFTA_OBJ_NAME], GFP_KERNEL);
+ if (!obj->name) {
+ err = -ENOMEM;
+ goto err2;
+ }
err = nft_trans_obj_add(&ctx, NFT_MSG_NEWOBJ, obj);
if (err < 0)
- goto err2;
+ goto err3;
list_add_tail_rcu(&obj->list, &table->objects);
table->use++;
return 0;
+err3:
+ kfree(obj->name);
err2:
if (obj->type->destroy)
obj->type->destroy(obj);
@@ -4631,6 +4637,7 @@ static void nft_obj_destroy(struct nft_object *obj)
obj->type->destroy(obj);
module_put(obj->type->owner);
+ kfree(obj->name);
kfree(obj);
}
--
2.13.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions
2017-07-27 14:56 [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions Phil Sutter
` (5 preceding siblings ...)
2017-07-27 14:56 ` [nf-next PATCH v3 6/6] netfilter: nf_tables: Allow object " Phil Sutter
@ 2017-07-31 17:17 ` Pablo Neira Ayuso
6 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2017-07-31 17:17 UTC (permalink / raw)
To: Phil Sutter; +Cc: netfilter-devel
On Thu, Jul 27, 2017 at 04:56:38PM +0200, Phil Sutter wrote:
> The following series lifts the tight restriction on name length of
> tables, chains, sets and objects. This is done by allocating memory for
> names dynamically, so there is no added overhead when reducing the
> restriction to a mere sanity level of 255 characters.
>
> The first patch removes a needless check discovered when discussing v2
> of this patch set.
>
> The second patch introduces nla_strdup() which aids in duplicating a
> string contained in a netlink attribute. It is used to replace the call
> to nla_strlcpy() when populating name fields.
>
> I've tested the series manually by creating tables, chains, sets and
> counter objects with long names and automated by running the py and
> shell testsuites of nftables repo. Also, kmemleak did not find anything
> nftables related.
Series applied, thanks Phil.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-07-31 17:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-27 14:56 [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 1/6] netfilter: nf_tables: No need to check chain existence when tracing Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 2/6] networking: Introduce nla_strdup() Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 3/6] netfilter: nf_tables: Allow table names of up to 255 chars Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 4/6] netfilter: nf_tables: Allow chain name " Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 5/6] netfilter: nf_tables: Allow set names " Phil Sutter
2017-07-27 14:56 ` [nf-next PATCH v3 6/6] netfilter: nf_tables: Allow object " Phil Sutter
2017-07-31 17:17 ` [nf-next PATCH v3 0/6] netfilter: nf_tables: Kill name length restrictions Pablo Neira Ayuso
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).