Skip to contentSkip to conversation

netfilter: nf_tables: limit maximum table name length to 32 bytes

PATCH In review Pablo Neira Ayuso 2 participants 5 messages Mar 5, 2015

Subsystems (2)
Networking netfilter
lore
Share & tools
lore

Pablo Neira Ayuso

author maintainer
#0 lore raw

PATCH 1/1 netfilter: nf_tables: limit maximum table name length to 32 bytes

Set the same as we use for chain names, it should be enough.

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
@Patrick: I'm planning to add per-table information after the name field, this will make it easier and this stays consistent to what we have in chains and sets (and other new object we'll add). I wouldn't expect people using larger names than this limit.

 include/net/netfilter/nf_tables.h        |    2 +-
 include/uapi/linux/netfilter/nf_tables.h |    1 +
 net/netfilter/nf_tables_api.c            |    7 ++++---
 3 files changed, 6 insertions(+), 4 deletions(-)
include/net/netfilter/nf_tables.h +1 -1
index 9eaaa78..f1e81d7 100644
@@ -584,7 +584,7 @@ struct nft_table {
 	u64				hgenerator;
 	u32				use;
 	u16				flags;
-	char				name[];
+	char				name[NFT_TABLE_MAXNAMELEN];
 };
 
 /**
include/uapi/linux/netfilter/nf_tables.h +1 -0
index 832bc46..b978393 100644
@@ -1,6 +1,7 @@
 #ifndef _LINUX_NF_TABLES_H
 #define _LINUX_NF_TABLES_H
 
+#define NFT_TABLE_MAXNAMELEN	32
 #define NFT_CHAIN_MAXNAMELEN	32
 #define NFT_USERDATA_MAXLEN	256
net/netfilter/nf_tables_api.c +4 -3
index a8c9462..9168e6c 100644
@@ -401,7 +401,8 @@ nf_tables_chain_type_lookup(const struct nft_af_info *afi,
 }
 
 static const struct nla_policy nft_table_policy[NFTA_TABLE_MAX + 1] = {
-	[NFTA_TABLE_NAME]	= { .type = NLA_STRING },
+	[NFTA_TABLE_NAME]	= { .type = NLA_STRING,
+				    .len = NFT_TABLE_MAXNAMELEN - 1 },
 	[NFTA_TABLE_FLAGS]	= { .type = NLA_U32 },
 };
 

@@ -686,13 +687,13 @@ static int nf_tables_newtable(struct sock *nlsk, struct sk_buff *skb,
 	if (!try_module_get(afi->owner))
 		return -EAFNOSUPPORT;
 
-	table = kzalloc(sizeof(*table) + nla_len(name), GFP_KERNEL);
+	table = kzalloc(sizeof(*table), GFP_KERNEL);
 	if (table == NULL) {
 		module_put(afi->owner);
 		return -ENOMEM;
 	}
 
-	nla_strlcpy(table->name, name, nla_len(name));
+	nla_strlcpy(table->name, name, NFT_TABLE_MAXNAMELEN);
 	INIT_LIST_HEAD(&table->chains);
 	INIT_LIST_HEAD(&table->sets);
 	table->flags = flags;
Signature · 1 line
1.7.10.4
#1 lore raw
Quoted 8 lines · Pablo Neira Ayuso

Set the same as we use for chain names, it should be enough.

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
@Patrick: I'm planning to add per-table information after the name field, this will make it easier and this stays consistent to what we have in chains and sets (and other new object we'll add). I wouldn't expect people using larger names than this limit.

What kind of information are we talking about?

author maintainer
#2 lore raw
Quoted 11 lines · Patrick McHardy
Quoted 8 lines · Pablo Neira Ayuso

Set the same as we use for chain names, it should be enough.

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
@Patrick: I'm planning to add per-table information after the name field, this will make it easier and this stays consistent to what we have in chains and sets (and other new object we'll add). I wouldn't expect people using larger names than this limit.

What kind of information are we talking about?

The hardware offload stuff, we'll need to keep a reference to the net_device *dev in the table, to call this from the commit path. But I can keep this in that batch if you prefer to have a larger view on this.

#3 lore raw
Quoted 11 lines · Pablo Neira Ayuso
Quoted 10 lines · Patrick McHardy
Quoted 8 lines · Pablo Neira Ayuso

Set the same as we use for chain names, it should be enough.

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
@Patrick: I'm planning to add per-table information after the name field, this will make it easier and this stays consistent to what we have in chains and sets (and other new object we'll add). I wouldn't expect people using larger names than this limit.

Quoted 6 lines
Quoted 1 line

What kind of information are we talking about?

The hardware offload stuff, we'll need to keep a reference to the net_device *dev in the table, to call this from the commit path. But I can keep this in that batch if you prefer to have a larger view on this.

Seems fine to me in either case, a larger name does indeed seem unnecessary and inconsistent.

BTW, what about my patches? :) Waiting for them (especially the fixes) to make their way to nf-next to continue ...

author maintainer
#4 lore raw
Quoted 14 lines · Patrick McHardy
Quoted 13 lines · Pablo Neira Ayuso
Quoted 11 lines · Patrick McHardy
Quoted 8 lines · Pablo Neira Ayuso

Set the same as we use for chain names, it should be enough.

Signed-off-by Pablo Neira Ayuso <[email protected]>

---
@Patrick: I'm planning to add per-table information after the name field, this will make it easier and this stays consistent to what we have in chains and sets (and other new object we'll add). I wouldn't expect people using larger names than this limit.

What kind of information are we talking about?

Quoted 7 lines
Quoted 4 lines

The hardware offload stuff, we'll need to keep a reference to the net_device *dev in the table, to call this from the commit path. But I can keep this in that batch if you prefer to have a larger view on this.

Seems fine to me in either case, a larger name does indeed seem unnecessary and inconsistent.

Thanks.

Quoted 2 lines

BTW, what about my patches? :) Waiting for them (especially the fixes) to make their way to nf-next to continue ...

Will send a batch with fixes today.