* [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