Netdev List
 help / color / mirror / Atom feed
* [PATCH 07/47] netfilter: x_tables: check standard verdicts in core
From: Pablo Neira Ayuso @ 2018-03-30 11:36 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330113729.18335-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Userspace must provide a valid verdict to the standard target.

The verdict can be either a jump (signed int > 0), or a return code.

Allowed return codes are either RETURN (pop from stack), NF_ACCEPT, DROP
and QUEUE (latter is allowed for legacy reasons).

Jump offsets (verdict > 0) are checked in more detail later on when
loop-detection is performed.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv4/netfilter/arp_tables.c |  5 -----
 net/ipv4/netfilter/ip_tables.c  |  5 -----
 net/ipv6/netfilter/ip6_tables.c |  5 -----
 net/netfilter/x_tables.c        | 49 ++++++++++++++++++++++++++++++++++++-----
 4 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index a0c7ce76879c..c9ffa884a4ee 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -334,11 +334,6 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
 			     t->verdict < 0) || visited) {
 				unsigned int oldpos, size;
 
-				if ((strcmp(t->target.u.user.name,
-					    XT_STANDARD_TARGET) == 0) &&
-				    t->verdict < -NF_MAX_VERDICT - 1)
-					return 0;
-
 				/* Return: backtrack through the last
 				 * big jump.
 				 */
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 4f7153e25e0b..c9b57a6bf96a 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -402,11 +402,6 @@ mark_source_chains(const struct xt_table_info *newinfo,
 			     t->verdict < 0) || visited) {
 				unsigned int oldpos, size;
 
-				if ((strcmp(t->target.u.user.name,
-					    XT_STANDARD_TARGET) == 0) &&
-				    t->verdict < -NF_MAX_VERDICT - 1)
-					return 0;
-
 				/* Return: backtrack through the last
 				   big jump. */
 				do {
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 6c44033decab..f46954221933 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -420,11 +420,6 @@ mark_source_chains(const struct xt_table_info *newinfo,
 			     t->verdict < 0) || visited) {
 				unsigned int oldpos, size;
 
-				if ((strcmp(t->target.u.user.name,
-					    XT_STANDARD_TARGET) == 0) &&
-				    t->verdict < -NF_MAX_VERDICT - 1)
-					return 0;
-
 				/* Return: backtrack through the last
 				   big jump. */
 				do {
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index d9deebe599ec..2e4d423e58e6 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -654,6 +654,31 @@ struct compat_xt_standard_target {
 	compat_uint_t verdict;
 };
 
+static bool verdict_ok(int verdict)
+{
+	if (verdict > 0)
+		return true;
+
+	if (verdict < 0) {
+		int v = -verdict - 1;
+
+		if (verdict == XT_RETURN)
+			return true;
+
+		switch (v) {
+		case NF_ACCEPT: return true;
+		case NF_DROP: return true;
+		case NF_QUEUE: return true;
+		default:
+			break;
+		}
+
+		return false;
+	}
+
+	return false;
+}
+
 int xt_compat_check_entry_offsets(const void *base, const char *elems,
 				  unsigned int target_offset,
 				  unsigned int next_offset)
@@ -675,9 +700,15 @@ int xt_compat_check_entry_offsets(const void *base, const char *elems,
 	if (target_offset + t->u.target_size > next_offset)
 		return -EINVAL;
 
-	if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 &&
-	    COMPAT_XT_ALIGN(target_offset + sizeof(struct compat_xt_standard_target)) != next_offset)
-		return -EINVAL;
+	if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0) {
+		const struct compat_xt_standard_target *st = (const void *)t;
+
+		if (COMPAT_XT_ALIGN(target_offset + sizeof(*st)) != next_offset)
+			return -EINVAL;
+
+		if (!verdict_ok(st->verdict))
+			return -EINVAL;
+	}
 
 	/* compat_xt_entry match has less strict alignment requirements,
 	 * otherwise they are identical.  In case of padding differences
@@ -757,9 +788,15 @@ int xt_check_entry_offsets(const void *base,
 	if (target_offset + t->u.target_size > next_offset)
 		return -EINVAL;
 
-	if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0 &&
-	    XT_ALIGN(target_offset + sizeof(struct xt_standard_target)) != next_offset)
-		return -EINVAL;
+	if (strcmp(t->u.user.name, XT_STANDARD_TARGET) == 0) {
+		const struct xt_standard_target *st = (const void *)t;
+
+		if (XT_ALIGN(target_offset + sizeof(*st)) != next_offset)
+			return -EINVAL;
+
+		if (!verdict_ok(st->verdict))
+			return -EINVAL;
+	}
 
 	return xt_check_entry_match(elems, base + target_offset,
 				    __alignof__(struct xt_entry_match));
-- 
2.11.0

^ permalink raw reply related

* [PATCH 08/47] netfilter: x_tables: check error target size too
From: Pablo Neira Ayuso @ 2018-03-30 11:36 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330113729.18335-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Check that userspace ERROR target (custom user-defined chains) match
expected format, and the chain name is null terminated.

This is irrelevant for kernel, but iptables itself relies on sane input
when it dumps rules from kernel.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/x_tables.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 2e4d423e58e6..f045bb4f7063 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -654,6 +654,11 @@ struct compat_xt_standard_target {
 	compat_uint_t verdict;
 };
 
+struct compat_xt_error_target {
+	struct compat_xt_entry_target t;
+	char errorname[XT_FUNCTION_MAXNAMELEN];
+};
+
 static bool verdict_ok(int verdict)
 {
 	if (verdict > 0)
@@ -679,6 +684,12 @@ static bool verdict_ok(int verdict)
 	return false;
 }
 
+static bool error_tg_ok(unsigned int usersize, unsigned int kernsize,
+			const char *msg, unsigned int msglen)
+{
+	return usersize == kernsize && strnlen(msg, msglen) < msglen;
+}
+
 int xt_compat_check_entry_offsets(const void *base, const char *elems,
 				  unsigned int target_offset,
 				  unsigned int next_offset)
@@ -708,6 +719,12 @@ int xt_compat_check_entry_offsets(const void *base, const char *elems,
 
 		if (!verdict_ok(st->verdict))
 			return -EINVAL;
+	} else if (strcmp(t->u.user.name, XT_ERROR_TARGET) == 0) {
+		const struct compat_xt_error_target *et = (const void *)t;
+
+		if (!error_tg_ok(t->u.target_size, sizeof(*et),
+				 et->errorname, sizeof(et->errorname)))
+			return -EINVAL;
 	}
 
 	/* compat_xt_entry match has less strict alignment requirements,
@@ -796,6 +813,12 @@ int xt_check_entry_offsets(const void *base,
 
 		if (!verdict_ok(st->verdict))
 			return -EINVAL;
+	} else if (strcmp(t->u.user.name, XT_ERROR_TARGET) == 0) {
+		const struct xt_error_target *et = (const void *)t;
+
+		if (!error_tg_ok(t->u.target_size, sizeof(*et),
+				 et->errorname, sizeof(et->errorname)))
+			return -EINVAL;
 	}
 
 	return xt_check_entry_match(elems, base + target_offset,
-- 
2.11.0

^ permalink raw reply related

* [PATCH 09/47] netfilter: x_tables: move hook entry checks into core
From: Pablo Neira Ayuso @ 2018-03-30 11:36 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330113729.18335-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Allow followup patch to change on location instead of three.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netfilter/x_tables.h |  2 ++
 net/ipv4/netfilter/arp_tables.c    | 13 +++----------
 net/ipv4/netfilter/ip_tables.c     | 13 +++----------
 net/ipv6/netfilter/ip6_tables.c    | 13 +++----------
 net/netfilter/x_tables.c           | 29 +++++++++++++++++++++++++++++
 5 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 1313b35c3ab7..fa0c19c328f1 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -281,6 +281,8 @@ int xt_check_entry_offsets(const void *base, const char *elems,
 			   unsigned int target_offset,
 			   unsigned int next_offset);
 
+int xt_check_table_hooks(const struct xt_table_info *info, unsigned int valid_hooks);
+
 unsigned int *xt_alloc_entry_offsets(unsigned int size);
 bool xt_find_jump_offset(const unsigned int *offsets,
 			 unsigned int target, unsigned int size);
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index c9ffa884a4ee..be5821215ea0 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -555,16 +555,9 @@ static int translate_table(struct xt_table_info *newinfo, void *entry0,
 	if (i != repl->num_entries)
 		goto out_free;
 
-	/* Check hooks all assigned */
-	for (i = 0; i < NF_ARP_NUMHOOKS; i++) {
-		/* Only hooks which are valid */
-		if (!(repl->valid_hooks & (1 << i)))
-			continue;
-		if (newinfo->hook_entry[i] == 0xFFFFFFFF)
-			goto out_free;
-		if (newinfo->underflow[i] == 0xFFFFFFFF)
-			goto out_free;
-	}
+	ret = xt_check_table_hooks(newinfo, repl->valid_hooks);
+	if (ret)
+		goto out_free;
 
 	if (!mark_source_chains(newinfo, repl->valid_hooks, entry0, offsets)) {
 		ret = -ELOOP;
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index c9b57a6bf96a..29bda9484a33 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -702,16 +702,9 @@ translate_table(struct net *net, struct xt_table_info *newinfo, void *entry0,
 	if (i != repl->num_entries)
 		goto out_free;
 
-	/* Check hooks all assigned */
-	for (i = 0; i < NF_INET_NUMHOOKS; i++) {
-		/* Only hooks which are valid */
-		if (!(repl->valid_hooks & (1 << i)))
-			continue;
-		if (newinfo->hook_entry[i] == 0xFFFFFFFF)
-			goto out_free;
-		if (newinfo->underflow[i] == 0xFFFFFFFF)
-			goto out_free;
-	}
+	ret = xt_check_table_hooks(newinfo, repl->valid_hooks);
+	if (ret)
+		goto out_free;
 
 	if (!mark_source_chains(newinfo, repl->valid_hooks, entry0, offsets)) {
 		ret = -ELOOP;
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index f46954221933..ba3776a4d305 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -720,16 +720,9 @@ translate_table(struct net *net, struct xt_table_info *newinfo, void *entry0,
 	if (i != repl->num_entries)
 		goto out_free;
 
-	/* Check hooks all assigned */
-	for (i = 0; i < NF_INET_NUMHOOKS; i++) {
-		/* Only hooks which are valid */
-		if (!(repl->valid_hooks & (1 << i)))
-			continue;
-		if (newinfo->hook_entry[i] == 0xFFFFFFFF)
-			goto out_free;
-		if (newinfo->underflow[i] == 0xFFFFFFFF)
-			goto out_free;
-	}
+	ret = xt_check_table_hooks(newinfo, repl->valid_hooks);
+	if (ret)
+		goto out_free;
 
 	if (!mark_source_chains(newinfo, repl->valid_hooks, entry0, offsets)) {
 		ret = -ELOOP;
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index f045bb4f7063..5d8ba89a8da8 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -518,6 +518,35 @@ static int xt_check_entry_match(const char *match, const char *target,
 	return 0;
 }
 
+/** xt_check_table_hooks - check hook entry points are sane
+ *
+ * @info xt_table_info to check
+ * @valid_hooks - hook entry points that we can enter from
+ *
+ * Validates that the hook entry and underflows points are set up.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int xt_check_table_hooks(const struct xt_table_info *info, unsigned int valid_hooks)
+{
+	unsigned int i;
+
+	BUILD_BUG_ON(ARRAY_SIZE(info->hook_entry) != ARRAY_SIZE(info->underflow));
+
+	for (i = 0; i < ARRAY_SIZE(info->hook_entry); i++) {
+		if (!(valid_hooks & (1 << i)))
+			continue;
+
+		if (info->hook_entry[i] == 0xFFFFFFFF)
+			return -EINVAL;
+		if (info->underflow[i] == 0xFFFFFFFF)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(xt_check_table_hooks);
+
 #ifdef CONFIG_COMPAT
 int xt_compat_add_offset(u_int8_t af, unsigned int offset, int delta)
 {
-- 
2.11.0

^ permalink raw reply related

* [PATCH 10/47] netfilter: x_tables: enforce unique and ascending entry points
From: Pablo Neira Ayuso @ 2018-03-30 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Florian Westphal <fw@strlen.de>

Harmless from kernel point of view, but iptables assumes that this is
true when decoding a ruleset.

iptables walks the dumped blob from kernel, and, for each entry that
creates a new chain it prints out rule/chain information.
Base chains (hook entry points) are thus only shown when they appear
in the rule blob.  One base chain that is referenced multiple times
in hook blob is then only printed once.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/x_tables.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 5d8ba89a8da8..4e6cbb38e616 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -529,10 +529,15 @@ static int xt_check_entry_match(const char *match, const char *target,
  */
 int xt_check_table_hooks(const struct xt_table_info *info, unsigned int valid_hooks)
 {
-	unsigned int i;
+	const char *err = "unsorted underflow";
+	unsigned int i, max_uflow, max_entry;
+	bool check_hooks = false;
 
 	BUILD_BUG_ON(ARRAY_SIZE(info->hook_entry) != ARRAY_SIZE(info->underflow));
 
+	max_entry = 0;
+	max_uflow = 0;
+
 	for (i = 0; i < ARRAY_SIZE(info->hook_entry); i++) {
 		if (!(valid_hooks & (1 << i)))
 			continue;
@@ -541,9 +546,33 @@ int xt_check_table_hooks(const struct xt_table_info *info, unsigned int valid_ho
 			return -EINVAL;
 		if (info->underflow[i] == 0xFFFFFFFF)
 			return -EINVAL;
+
+		if (check_hooks) {
+			if (max_uflow > info->underflow[i])
+				goto error;
+
+			if (max_uflow == info->underflow[i]) {
+				err = "duplicate underflow";
+				goto error;
+			}
+			if (max_entry > info->hook_entry[i]) {
+				err = "unsorted entry";
+				goto error;
+			}
+			if (max_entry == info->hook_entry[i]) {
+				err = "duplicate entry";
+				goto error;
+			}
+		}
+		max_entry = info->hook_entry[i];
+		max_uflow = info->underflow[i];
+		check_hooks = true;
 	}
 
 	return 0;
+error:
+	pr_err_ratelimited("%s at hook %d\n", err, i);
+	return -EINVAL;
 }
 EXPORT_SYMBOL(xt_check_table_hooks);
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH 11/47] netfilter: x_tables: cap allocations at 512 mbyte
From: Pablo Neira Ayuso @ 2018-03-30 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330113914.18431-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Arbitrary limit, however, this still allows huge rulesets
(> 1 million rules).  This helps with automated fuzzer as it prevents
oom-killer invocation.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/x_tables.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 4e6cbb38e616..dc68ac49614a 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -40,6 +40,7 @@ MODULE_AUTHOR("Harald Welte <laforge@netfilter.org>");
 MODULE_DESCRIPTION("{ip,ip6,arp,eb}_tables backend module");
 
 #define XT_PCPU_BLOCK_SIZE 4096
+#define XT_MAX_TABLE_SIZE	(512 * 1024 * 1024)
 
 struct compat_delta {
 	unsigned int offset; /* offset in kernel */
@@ -1117,7 +1118,7 @@ struct xt_table_info *xt_alloc_table_info(unsigned int size)
 	struct xt_table_info *info = NULL;
 	size_t sz = sizeof(*info) + size;
 
-	if (sz < sizeof(*info))
+	if (sz < sizeof(*info) || sz >= XT_MAX_TABLE_SIZE)
 		return NULL;
 
 	/* __GFP_NORETRY is not fully supported by kvmalloc but it should
-- 
2.11.0

^ permalink raw reply related

* A question about inet_csk_bind_conflict
From: martin zhang @ 2018-03-30 11:39 UTC (permalink / raw)
  To: davem, Eric Dumazet; +Cc: netdev

HI all,

I have a question about tcp bind check function:inet_csk_bind_conflict.

My case:

192.168.56.101:37818    ==> 192.168.56.193:22
On host A (tcp client: 192.168.56.101), there is  a tcp connection to
server B(192.168.56.193, tcp server).

I want run a tcp server on A, and listen/bind tcp port 37818, but it failed.
I check the kernel source and found it is stopped by inet_csk_bind_conflict.

I think in this case, it should success.
Because,
1. when a tcp packet arrives network stack, we firstly look the
established/timewait socket,
and then search for listen socket.
2. peer side(192.168.56.193:22), is a tcp server. It will never
connect to192.168.56.101:37818, with 192.168.56.193:22.

I don’t  understand why the kernel fail at inet_csk_bind_conflict,
I wonder if I ignore some case or  it is a bug?

Thanks!

Martin

^ permalink raw reply

* [PATCH 12/47] netfilter: x_tables: limit allocation requests for blob rule heads
From: Pablo Neira Ayuso @ 2018-03-30 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330113914.18431-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

This is a very conservative limit (134217728 rules), but good
enough to not trigger frequent oom from syzkaller.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/x_tables.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index dc68ac49614a..01f8e122e74e 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -894,6 +894,9 @@ EXPORT_SYMBOL(xt_check_entry_offsets);
  */
 unsigned int *xt_alloc_entry_offsets(unsigned int size)
 {
+	if (size > XT_MAX_TABLE_SIZE / sizeof(unsigned int))
+		return NULL;
+
 	return kvmalloc_array(size, sizeof(unsigned int), GFP_KERNEL | __GFP_ZERO);
 
 }
-- 
2.11.0

^ permalink raw reply related

* [PATCH 13/47] netfilter: x_tables: add counters allocation wrapper
From: Pablo Neira Ayuso @ 2018-03-30 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330113914.18431-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

allows to have size checks in a single spot.
This is supposed to reduce oom situations when fuzz-testing xtables.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netfilter/x_tables.h |  1 +
 net/ipv4/netfilter/arp_tables.c    |  2 +-
 net/ipv4/netfilter/ip_tables.c     |  2 +-
 net/ipv6/netfilter/ip6_tables.c    |  2 +-
 net/netfilter/x_tables.c           | 15 +++++++++++++++
 5 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index fa0c19c328f1..0bd93c589a8c 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -301,6 +301,7 @@ int xt_data_to_user(void __user *dst, const void *src,
 
 void *xt_copy_counters_from_user(const void __user *user, unsigned int len,
 				 struct xt_counters_info *info, bool compat);
+struct xt_counters *xt_counters_alloc(unsigned int counters);
 
 struct xt_table *xt_register_table(struct net *net,
 				   const struct xt_table *table,
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index be5821215ea0..82ba09b50fdb 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -883,7 +883,7 @@ static int __do_replace(struct net *net, const char *name,
 	struct arpt_entry *iter;
 
 	ret = 0;
-	counters = vzalloc(num_counters * sizeof(struct xt_counters));
+	counters = xt_counters_alloc(num_counters);
 	if (!counters) {
 		ret = -ENOMEM;
 		goto out;
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 29bda9484a33..4901ca6c3e09 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1045,7 +1045,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
 	struct ipt_entry *iter;
 
 	ret = 0;
-	counters = vzalloc(num_counters * sizeof(struct xt_counters));
+	counters = xt_counters_alloc(num_counters);
 	if (!counters) {
 		ret = -ENOMEM;
 		goto out;
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index ba3776a4d305..e84cec49b60f 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -1063,7 +1063,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks,
 	struct ip6t_entry *iter;
 
 	ret = 0;
-	counters = vzalloc(num_counters * sizeof(struct xt_counters));
+	counters = xt_counters_alloc(num_counters);
 	if (!counters) {
 		ret = -ENOMEM;
 		goto out;
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 01f8e122e74e..82b1f8f52ac6 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -1290,6 +1290,21 @@ static int xt_jumpstack_alloc(struct xt_table_info *i)
 	return 0;
 }
 
+struct xt_counters *xt_counters_alloc(unsigned int counters)
+{
+	struct xt_counters *mem;
+
+	if (counters == 0 || counters > INT_MAX / sizeof(*mem))
+		return NULL;
+
+	counters *= sizeof(*mem);
+	if (counters > XT_MAX_TABLE_SIZE)
+		return NULL;
+
+	return vzalloc(counters);
+}
+EXPORT_SYMBOL(xt_counters_alloc);
+
 struct xt_table_info *
 xt_replace_table(struct xt_table *table,
 	      unsigned int num_counters,
-- 
2.11.0

^ permalink raw reply related

* [PATCH 14/47] netfilter: compat: prepare xt_compat_init_offsets to return errors
From: Pablo Neira Ayuso @ 2018-03-30 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330113914.18431-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

should have no impact, function still always returns 0.
This patch is only to ease review.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netfilter/x_tables.h |  2 +-
 net/bridge/netfilter/ebtables.c    | 10 ++++++++--
 net/ipv4/netfilter/arp_tables.c    | 10 +++++++---
 net/ipv4/netfilter/ip_tables.c     |  8 ++++++--
 net/ipv6/netfilter/ip6_tables.c    | 10 +++++++---
 net/netfilter/x_tables.c           |  4 +++-
 6 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 0bd93c589a8c..7bd896dc78df 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -510,7 +510,7 @@ void xt_compat_unlock(u_int8_t af);
 
 int xt_compat_add_offset(u_int8_t af, unsigned int offset, int delta);
 void xt_compat_flush_offsets(u_int8_t af);
-void xt_compat_init_offsets(u_int8_t af, unsigned int number);
+int xt_compat_init_offsets(u8 af, unsigned int number);
 int xt_compat_calc_jump(u_int8_t af, unsigned int offset);
 
 int xt_compat_match_offset(const struct xt_match *match);
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 02c4b409d317..217aa79f7b2a 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1819,10 +1819,14 @@ static int compat_table_info(const struct ebt_table_info *info,
 {
 	unsigned int size = info->entries_size;
 	const void *entries = info->entries;
+	int ret;
 
 	newinfo->entries_size = size;
 
-	xt_compat_init_offsets(NFPROTO_BRIDGE, info->nentries);
+	ret = xt_compat_init_offsets(NFPROTO_BRIDGE, info->nentries);
+	if (ret)
+		return ret;
+
 	return EBT_ENTRY_ITERATE(entries, size, compat_calc_entry, info,
 							entries, newinfo);
 }
@@ -2245,7 +2249,9 @@ static int compat_do_replace(struct net *net, void __user *user,
 
 	xt_compat_lock(NFPROTO_BRIDGE);
 
-	xt_compat_init_offsets(NFPROTO_BRIDGE, tmp.nentries);
+	ret = xt_compat_init_offsets(NFPROTO_BRIDGE, tmp.nentries);
+	if (ret < 0)
+		goto out_unlock;
 	ret = compat_copy_entries(entries_tmp, tmp.entries_size, &state);
 	if (ret < 0)
 		goto out_unlock;
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 82ba09b50fdb..aaafdbd15ad3 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -769,7 +769,9 @@ static int compat_table_info(const struct xt_table_info *info,
 	memcpy(newinfo, info, offsetof(struct xt_table_info, entries));
 	newinfo->initial_entries = 0;
 	loc_cpu_entry = info->entries;
-	xt_compat_init_offsets(NFPROTO_ARP, info->number);
+	ret = xt_compat_init_offsets(NFPROTO_ARP, info->number);
+	if (ret)
+		return ret;
 	xt_entry_foreach(iter, loc_cpu_entry, info->size) {
 		ret = compat_calc_entry(iter, info, loc_cpu_entry, newinfo);
 		if (ret != 0)
@@ -1156,7 +1158,7 @@ static int translate_compat_table(struct xt_table_info **pinfo,
 	struct compat_arpt_entry *iter0;
 	struct arpt_replace repl;
 	unsigned int size;
-	int ret = 0;
+	int ret;
 
 	info = *pinfo;
 	entry0 = *pentry0;
@@ -1165,7 +1167,9 @@ static int translate_compat_table(struct xt_table_info **pinfo,
 
 	j = 0;
 	xt_compat_lock(NFPROTO_ARP);
-	xt_compat_init_offsets(NFPROTO_ARP, compatr->num_entries);
+	ret = xt_compat_init_offsets(NFPROTO_ARP, compatr->num_entries);
+	if (ret)
+		goto out_unlock;
 	/* Walk through entries, checking offsets. */
 	xt_entry_foreach(iter0, entry0, compatr->size) {
 		ret = check_compat_entry_size_and_hooks(iter0, info, &size,
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 4901ca6c3e09..f9063513f9d1 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -933,7 +933,9 @@ static int compat_table_info(const struct xt_table_info *info,
 	memcpy(newinfo, info, offsetof(struct xt_table_info, entries));
 	newinfo->initial_entries = 0;
 	loc_cpu_entry = info->entries;
-	xt_compat_init_offsets(AF_INET, info->number);
+	ret = xt_compat_init_offsets(AF_INET, info->number);
+	if (ret)
+		return ret;
 	xt_entry_foreach(iter, loc_cpu_entry, info->size) {
 		ret = compat_calc_entry(iter, info, loc_cpu_entry, newinfo);
 		if (ret != 0)
@@ -1407,7 +1409,9 @@ translate_compat_table(struct net *net,
 
 	j = 0;
 	xt_compat_lock(AF_INET);
-	xt_compat_init_offsets(AF_INET, compatr->num_entries);
+	ret = xt_compat_init_offsets(AF_INET, compatr->num_entries);
+	if (ret)
+		goto out_unlock;
 	/* Walk through entries, checking offsets. */
 	xt_entry_foreach(iter0, entry0, compatr->size) {
 		ret = check_compat_entry_size_and_hooks(iter0, info, &size,
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index e84cec49b60f..3c36a4c77f29 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -950,7 +950,9 @@ static int compat_table_info(const struct xt_table_info *info,
 	memcpy(newinfo, info, offsetof(struct xt_table_info, entries));
 	newinfo->initial_entries = 0;
 	loc_cpu_entry = info->entries;
-	xt_compat_init_offsets(AF_INET6, info->number);
+	ret = xt_compat_init_offsets(AF_INET6, info->number);
+	if (ret)
+		return ret;
 	xt_entry_foreach(iter, loc_cpu_entry, info->size) {
 		ret = compat_calc_entry(iter, info, loc_cpu_entry, newinfo);
 		if (ret != 0)
@@ -1414,7 +1416,7 @@ translate_compat_table(struct net *net,
 	struct compat_ip6t_entry *iter0;
 	struct ip6t_replace repl;
 	unsigned int size;
-	int ret = 0;
+	int ret;
 
 	info = *pinfo;
 	entry0 = *pentry0;
@@ -1423,7 +1425,9 @@ translate_compat_table(struct net *net,
 
 	j = 0;
 	xt_compat_lock(AF_INET6);
-	xt_compat_init_offsets(AF_INET6, compatr->num_entries);
+	ret = xt_compat_init_offsets(AF_INET6, compatr->num_entries);
+	if (ret)
+		goto out_unlock;
 	/* Walk through entries, checking offsets. */
 	xt_entry_foreach(iter0, entry0, compatr->size) {
 		ret = check_compat_entry_size_and_hooks(iter0, info, &size,
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 82b1f8f52ac6..e878c85a9268 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -632,10 +632,12 @@ int xt_compat_calc_jump(u_int8_t af, unsigned int offset)
 }
 EXPORT_SYMBOL_GPL(xt_compat_calc_jump);
 
-void xt_compat_init_offsets(u_int8_t af, unsigned int number)
+int xt_compat_init_offsets(u8 af, unsigned int number)
 {
 	xt[af].number = number;
 	xt[af].cur = 0;
+
+	return 0;
 }
 EXPORT_SYMBOL(xt_compat_init_offsets);
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH 15/47] netfilter: compat: reject huge allocation requests
From: Pablo Neira Ayuso @ 2018-03-30 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330113914.18431-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

no need to bother even trying to allocating huge compat offset arrays,
such ruleset is rejected later on anyway becaus we refuse to allocate
overly large rule blobs.

However, compat translation happens before blob allocation, so we should
add a check there too.

This is supposed to help with fuzzing by avoiding oom-killer.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/x_tables.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index e878c85a9268..33724b08b8f0 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -582,14 +582,8 @@ int xt_compat_add_offset(u_int8_t af, unsigned int offset, int delta)
 {
 	struct xt_af *xp = &xt[af];
 
-	if (!xp->compat_tab) {
-		if (!xp->number)
-			return -EINVAL;
-		xp->compat_tab = vmalloc(sizeof(struct compat_delta) * xp->number);
-		if (!xp->compat_tab)
-			return -ENOMEM;
-		xp->cur = 0;
-	}
+	if (WARN_ON(!xp->compat_tab))
+		return -ENOMEM;
 
 	if (xp->cur >= xp->number)
 		return -EINVAL;
@@ -634,6 +628,22 @@ EXPORT_SYMBOL_GPL(xt_compat_calc_jump);
 
 int xt_compat_init_offsets(u8 af, unsigned int number)
 {
+	size_t mem;
+
+	if (!number || number > (INT_MAX / sizeof(struct compat_delta)))
+		return -EINVAL;
+
+	if (WARN_ON(xt[af].compat_tab))
+		return -EINVAL;
+
+	mem = sizeof(struct compat_delta) * number;
+	if (mem > XT_MAX_TABLE_SIZE)
+		return -ENOMEM;
+
+	xt[af].compat_tab = vmalloc(mem);
+	if (!xt[af].compat_tab)
+		return -ENOMEM;
+
 	xt[af].number = number;
 	xt[af].cur = 0;
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH 16/47] netfilter: x_tables: make sure compat af mutex is held
From: Pablo Neira Ayuso @ 2018-03-30 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330113914.18431-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/x_tables.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 33724b08b8f0..7521e8a72c06 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -582,6 +582,8 @@ int xt_compat_add_offset(u_int8_t af, unsigned int offset, int delta)
 {
 	struct xt_af *xp = &xt[af];
 
+	WARN_ON(!mutex_is_locked(&xt[af].compat_mutex));
+
 	if (WARN_ON(!xp->compat_tab))
 		return -ENOMEM;
 
@@ -599,6 +601,8 @@ EXPORT_SYMBOL_GPL(xt_compat_add_offset);
 
 void xt_compat_flush_offsets(u_int8_t af)
 {
+	WARN_ON(!mutex_is_locked(&xt[af].compat_mutex));
+
 	if (xt[af].compat_tab) {
 		vfree(xt[af].compat_tab);
 		xt[af].compat_tab = NULL;
@@ -630,6 +634,8 @@ int xt_compat_init_offsets(u8 af, unsigned int number)
 {
 	size_t mem;
 
+	WARN_ON(!mutex_is_locked(&xt[af].compat_mutex));
+
 	if (!number || number > (INT_MAX / sizeof(struct compat_delta)))
 		return -EINVAL;
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH 17/47] netfilter: x_tables: ensure last rule in base chain matches underflow/policy
From: Pablo Neira Ayuso @ 2018-03-30 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330113914.18431-1-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

Harmless from kernel point of view, but again iptables assumes that
this is true when decoding ruleset coming from kernel.

If a (syzkaller generated) ruleset doesn't have the underflow/policy
stored as the last rule in the base chain, then iptables will abort()
because it doesn't find the chain policy.

libiptc assumes that the policy is the last rule in the basechain, which
is only true for iptables-generated rulesets.

Unfortunately this needs code duplication -- the functions need the
struct layout of the rule head, but that is different for
ip/ip6/arptables.

NB: pr_warn could be pr_debug but in case this break rulesets somehow its
useful to know why blob was rejected.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv4/netfilter/arp_tables.c | 17 ++++++++++++++++-
 net/ipv4/netfilter/ip_tables.c  | 17 ++++++++++++++++-
 net/ipv6/netfilter/ip6_tables.c | 17 ++++++++++++++++-
 3 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index aaafdbd15ad3..f366ff1cfc19 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -309,10 +309,13 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
 	for (hook = 0; hook < NF_ARP_NUMHOOKS; hook++) {
 		unsigned int pos = newinfo->hook_entry[hook];
 		struct arpt_entry *e = entry0 + pos;
+		unsigned int last_pos, depth;
 
 		if (!(valid_hooks & (1 << hook)))
 			continue;
 
+		depth = 0;
+		last_pos = pos;
 		/* Set initial back pointer. */
 		e->counters.pcnt = pos;
 
@@ -343,6 +346,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
 					pos = e->counters.pcnt;
 					e->counters.pcnt = 0;
 
+					if (depth)
+						--depth;
 					/* We're at the start. */
 					if (pos == oldpos)
 						goto next;
@@ -367,6 +372,9 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
 					if (!xt_find_jump_offset(offsets, newpos,
 								 newinfo->number))
 						return 0;
+
+					if (entry0 + newpos != arpt_next_entry(e))
+						++depth;
 				} else {
 					/* ... this is a fallthru */
 					newpos = pos + e->next_offset;
@@ -377,8 +385,15 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
 				e->counters.pcnt = pos;
 				pos = newpos;
 			}
+			if (depth == 0)
+				last_pos = pos;
+		}
+next:
+		if (last_pos != newinfo->underflow[hook]) {
+			pr_err_ratelimited("last base chain position %u doesn't match underflow %u (hook %u)\n",
+					   last_pos, newinfo->underflow[hook], hook);
+			return 0;
 		}
-next:		;
 	}
 	return 1;
 }
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index f9063513f9d1..2362ca2c9e0c 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -378,10 +378,13 @@ mark_source_chains(const struct xt_table_info *newinfo,
 	for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) {
 		unsigned int pos = newinfo->hook_entry[hook];
 		struct ipt_entry *e = entry0 + pos;
+		unsigned int last_pos, depth;
 
 		if (!(valid_hooks & (1 << hook)))
 			continue;
 
+		depth = 0;
+		last_pos = pos;
 		/* Set initial back pointer. */
 		e->counters.pcnt = pos;
 
@@ -410,6 +413,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
 					pos = e->counters.pcnt;
 					e->counters.pcnt = 0;
 
+					if (depth)
+						--depth;
 					/* We're at the start. */
 					if (pos == oldpos)
 						goto next;
@@ -434,6 +439,9 @@ mark_source_chains(const struct xt_table_info *newinfo,
 					if (!xt_find_jump_offset(offsets, newpos,
 								 newinfo->number))
 						return 0;
+
+					if (entry0 + newpos != ipt_next_entry(e))
+						++depth;
 				} else {
 					/* ... this is a fallthru */
 					newpos = pos + e->next_offset;
@@ -444,8 +452,15 @@ mark_source_chains(const struct xt_table_info *newinfo,
 				e->counters.pcnt = pos;
 				pos = newpos;
 			}
+			if (depth == 0)
+				last_pos = pos;
+		}
+next:
+		if (last_pos != newinfo->underflow[hook]) {
+			pr_err_ratelimited("last base chain position %u doesn't match underflow %u (hook %u)\n",
+					   last_pos, newinfo->underflow[hook], hook);
+			return 0;
 		}
-next:		;
 	}
 	return 1;
 }
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 3c36a4c77f29..004508753abc 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -396,10 +396,13 @@ mark_source_chains(const struct xt_table_info *newinfo,
 	for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) {
 		unsigned int pos = newinfo->hook_entry[hook];
 		struct ip6t_entry *e = entry0 + pos;
+		unsigned int last_pos, depth;
 
 		if (!(valid_hooks & (1 << hook)))
 			continue;
 
+		depth = 0;
+		last_pos = pos;
 		/* Set initial back pointer. */
 		e->counters.pcnt = pos;
 
@@ -428,6 +431,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
 					pos = e->counters.pcnt;
 					e->counters.pcnt = 0;
 
+					if (depth)
+						--depth;
 					/* We're at the start. */
 					if (pos == oldpos)
 						goto next;
@@ -452,6 +457,9 @@ mark_source_chains(const struct xt_table_info *newinfo,
 					if (!xt_find_jump_offset(offsets, newpos,
 								 newinfo->number))
 						return 0;
+
+					if (entry0 + newpos != ip6t_next_entry(e))
+						++depth;
 				} else {
 					/* ... this is a fallthru */
 					newpos = pos + e->next_offset;
@@ -462,8 +470,15 @@ mark_source_chains(const struct xt_table_info *newinfo,
 				e->counters.pcnt = pos;
 				pos = newpos;
 			}
+			if (depth == 0)
+				last_pos = pos;
+		}
+next:
+		if (last_pos != newinfo->underflow[hook]) {
+			pr_err_ratelimited("last base chain position %u doesn't match underflow %u (hook %u)\n",
+					   last_pos, newinfo->underflow[hook], hook);
+			return 0;
 		}
-next:		;
 	}
 	return 1;
 }
-- 
2.11.0

^ permalink raw reply related

* [PATCH 18/47] netfilter: make xt_rateest hash table per net
From: Pablo Neira Ayuso @ 2018-03-30 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330113914.18431-1-pablo@netfilter.org>

From: Cong Wang <xiyou.wangcong@gmail.com>

As suggested by Eric, we need to make the xt_rateest
hash table and its lock per netns to reduce lock
contentions.

Cc: Florian Westphal <fw@strlen.de>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/xt_rateest.h |  4 +-
 net/netfilter/xt_RATEEST.c         | 91 +++++++++++++++++++++++++++-----------
 net/netfilter/xt_rateest.c         | 10 ++---
 3 files changed, 72 insertions(+), 33 deletions(-)

diff --git a/include/net/netfilter/xt_rateest.h b/include/net/netfilter/xt_rateest.h
index b1db13772554..832ab69efda5 100644
--- a/include/net/netfilter/xt_rateest.h
+++ b/include/net/netfilter/xt_rateest.h
@@ -21,7 +21,7 @@ struct xt_rateest {
 	struct net_rate_estimator __rcu *rate_est;
 };
 
-struct xt_rateest *xt_rateest_lookup(const char *name);
-void xt_rateest_put(struct xt_rateest *est);
+struct xt_rateest *xt_rateest_lookup(struct net *net, const char *name);
+void xt_rateest_put(struct net *net, struct xt_rateest *est);
 
 #endif /* _XT_RATEEST_H */
diff --git a/net/netfilter/xt_RATEEST.c b/net/netfilter/xt_RATEEST.c
index 141c295191f6..dec843cadf46 100644
--- a/net/netfilter/xt_RATEEST.c
+++ b/net/netfilter/xt_RATEEST.c
@@ -14,15 +14,21 @@
 #include <linux/slab.h>
 #include <net/gen_stats.h>
 #include <net/netlink.h>
+#include <net/netns/generic.h>
 
 #include <linux/netfilter/x_tables.h>
 #include <linux/netfilter/xt_RATEEST.h>
 #include <net/netfilter/xt_rateest.h>
 
-static DEFINE_MUTEX(xt_rateest_mutex);
-
 #define RATEEST_HSIZE	16
-static struct hlist_head rateest_hash[RATEEST_HSIZE] __read_mostly;
+
+struct xt_rateest_net {
+	struct mutex hash_lock;
+	struct hlist_head hash[RATEEST_HSIZE];
+};
+
+static unsigned int xt_rateest_id;
+
 static unsigned int jhash_rnd __read_mostly;
 
 static unsigned int xt_rateest_hash(const char *name)
@@ -31,21 +37,23 @@ static unsigned int xt_rateest_hash(const char *name)
 	       (RATEEST_HSIZE - 1);
 }
 
-static void xt_rateest_hash_insert(struct xt_rateest *est)
+static void xt_rateest_hash_insert(struct xt_rateest_net *xn,
+				   struct xt_rateest *est)
 {
 	unsigned int h;
 
 	h = xt_rateest_hash(est->name);
-	hlist_add_head(&est->list, &rateest_hash[h]);
+	hlist_add_head(&est->list, &xn->hash[h]);
 }
 
-static struct xt_rateest *__xt_rateest_lookup(const char *name)
+static struct xt_rateest *__xt_rateest_lookup(struct xt_rateest_net *xn,
+					      const char *name)
 {
 	struct xt_rateest *est;
 	unsigned int h;
 
 	h = xt_rateest_hash(name);
-	hlist_for_each_entry(est, &rateest_hash[h], list) {
+	hlist_for_each_entry(est, &xn->hash[h], list) {
 		if (strcmp(est->name, name) == 0) {
 			est->refcnt++;
 			return est;
@@ -55,20 +63,23 @@ static struct xt_rateest *__xt_rateest_lookup(const char *name)
 	return NULL;
 }
 
-struct xt_rateest *xt_rateest_lookup(const char *name)
+struct xt_rateest *xt_rateest_lookup(struct net *net, const char *name)
 {
+	struct xt_rateest_net *xn = net_generic(net, xt_rateest_id);
 	struct xt_rateest *est;
 
-	mutex_lock(&xt_rateest_mutex);
-	est = __xt_rateest_lookup(name);
-	mutex_unlock(&xt_rateest_mutex);
+	mutex_lock(&xn->hash_lock);
+	est = __xt_rateest_lookup(xn, name);
+	mutex_unlock(&xn->hash_lock);
 	return est;
 }
 EXPORT_SYMBOL_GPL(xt_rateest_lookup);
 
-void xt_rateest_put(struct xt_rateest *est)
+void xt_rateest_put(struct net *net, struct xt_rateest *est)
 {
-	mutex_lock(&xt_rateest_mutex);
+	struct xt_rateest_net *xn = net_generic(net, xt_rateest_id);
+
+	mutex_lock(&xn->hash_lock);
 	if (--est->refcnt == 0) {
 		hlist_del(&est->list);
 		gen_kill_estimator(&est->rate_est);
@@ -78,7 +89,7 @@ void xt_rateest_put(struct xt_rateest *est)
 		 */
 		kfree_rcu(est, rcu);
 	}
-	mutex_unlock(&xt_rateest_mutex);
+	mutex_unlock(&xn->hash_lock);
 }
 EXPORT_SYMBOL_GPL(xt_rateest_put);
 
@@ -98,6 +109,7 @@ xt_rateest_tg(struct sk_buff *skb, const struct xt_action_param *par)
 
 static int xt_rateest_tg_checkentry(const struct xt_tgchk_param *par)
 {
+	struct xt_rateest_net *xn = net_generic(par->net, xt_rateest_id);
 	struct xt_rateest_target_info *info = par->targinfo;
 	struct xt_rateest *est;
 	struct {
@@ -108,10 +120,10 @@ static int xt_rateest_tg_checkentry(const struct xt_tgchk_param *par)
 
 	net_get_random_once(&jhash_rnd, sizeof(jhash_rnd));
 
-	mutex_lock(&xt_rateest_mutex);
-	est = __xt_rateest_lookup(info->name);
+	mutex_lock(&xn->hash_lock);
+	est = __xt_rateest_lookup(xn, info->name);
 	if (est) {
-		mutex_unlock(&xt_rateest_mutex);
+		mutex_unlock(&xn->hash_lock);
 		/*
 		 * If estimator parameters are specified, they must match the
 		 * existing estimator.
@@ -119,7 +131,7 @@ static int xt_rateest_tg_checkentry(const struct xt_tgchk_param *par)
 		if ((!info->interval && !info->ewma_log) ||
 		    (info->interval != est->params.interval ||
 		     info->ewma_log != est->params.ewma_log)) {
-			xt_rateest_put(est);
+			xt_rateest_put(par->net, est);
 			return -EINVAL;
 		}
 		info->est = est;
@@ -148,14 +160,14 @@ static int xt_rateest_tg_checkentry(const struct xt_tgchk_param *par)
 		goto err2;
 
 	info->est = est;
-	xt_rateest_hash_insert(est);
-	mutex_unlock(&xt_rateest_mutex);
+	xt_rateest_hash_insert(xn, est);
+	mutex_unlock(&xn->hash_lock);
 	return 0;
 
 err2:
 	kfree(est);
 err1:
-	mutex_unlock(&xt_rateest_mutex);
+	mutex_unlock(&xn->hash_lock);
 	return ret;
 }
 
@@ -163,7 +175,7 @@ static void xt_rateest_tg_destroy(const struct xt_tgdtor_param *par)
 {
 	struct xt_rateest_target_info *info = par->targinfo;
 
-	xt_rateest_put(info->est);
+	xt_rateest_put(par->net, info->est);
 }
 
 static struct xt_target xt_rateest_tg_reg __read_mostly = {
@@ -178,19 +190,46 @@ static struct xt_target xt_rateest_tg_reg __read_mostly = {
 	.me         = THIS_MODULE,
 };
 
-static int __init xt_rateest_tg_init(void)
+static __net_init int xt_rateest_net_init(struct net *net)
+{
+	struct xt_rateest_net *xn = net_generic(net, xt_rateest_id);
+	int i;
+
+	mutex_init(&xn->hash_lock);
+	for (i = 0; i < ARRAY_SIZE(xn->hash); i++)
+		INIT_HLIST_HEAD(&xn->hash[i]);
+	return 0;
+}
+
+static void __net_exit xt_rateest_net_exit(struct net *net)
 {
-	unsigned int i;
+	struct xt_rateest_net *xn = net_generic(net, xt_rateest_id);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(xn->hash); i++)
+		WARN_ON_ONCE(!hlist_empty(&xn->hash[i]));
+}
 
-	for (i = 0; i < ARRAY_SIZE(rateest_hash); i++)
-		INIT_HLIST_HEAD(&rateest_hash[i]);
+static struct pernet_operations xt_rateest_net_ops = {
+	.init = xt_rateest_net_init,
+	.exit = xt_rateest_net_exit,
+	.id   = &xt_rateest_id,
+	.size = sizeof(struct xt_rateest_net),
+};
+
+static int __init xt_rateest_tg_init(void)
+{
+	int err = register_pernet_subsys(&xt_rateest_net_ops);
 
+	if (err)
+		return err;
 	return xt_register_target(&xt_rateest_tg_reg);
 }
 
 static void __exit xt_rateest_tg_fini(void)
 {
 	xt_unregister_target(&xt_rateest_tg_reg);
+	unregister_pernet_subsys(&xt_rateest_net_ops);
 }
 
 
diff --git a/net/netfilter/xt_rateest.c b/net/netfilter/xt_rateest.c
index 755d2f6693a2..bf77326861af 100644
--- a/net/netfilter/xt_rateest.c
+++ b/net/netfilter/xt_rateest.c
@@ -95,13 +95,13 @@ static int xt_rateest_mt_checkentry(const struct xt_mtchk_param *par)
 	}
 
 	ret  = -ENOENT;
-	est1 = xt_rateest_lookup(info->name1);
+	est1 = xt_rateest_lookup(par->net, info->name1);
 	if (!est1)
 		goto err1;
 
 	est2 = NULL;
 	if (info->flags & XT_RATEEST_MATCH_REL) {
-		est2 = xt_rateest_lookup(info->name2);
+		est2 = xt_rateest_lookup(par->net, info->name2);
 		if (!est2)
 			goto err2;
 	}
@@ -111,7 +111,7 @@ static int xt_rateest_mt_checkentry(const struct xt_mtchk_param *par)
 	return 0;
 
 err2:
-	xt_rateest_put(est1);
+	xt_rateest_put(par->net, est1);
 err1:
 	return ret;
 }
@@ -120,9 +120,9 @@ static void xt_rateest_mt_destroy(const struct xt_mtdtor_param *par)
 {
 	struct xt_rateest_match_info *info = par->matchinfo;
 
-	xt_rateest_put(info->est1);
+	xt_rateest_put(par->net, info->est1);
 	if (info->est2)
-		xt_rateest_put(info->est2);
+		xt_rateest_put(par->net, info->est2);
 }
 
 static struct xt_match xt_rateest_mt_reg __read_mostly = {
-- 
2.11.0

^ permalink raw reply related

* [PATCH 19/47] netfilter: xt_limit: Spelling s/maxmum/maximum/
From: Pablo Neira Ayuso @ 2018-03-30 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330113914.18431-1-pablo@netfilter.org>

From: Geert Uytterhoeven <geert+renesas@glider.be>

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/xt_limit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/xt_limit.c b/net/netfilter/xt_limit.c
index 55d18cd67635..9f098ecb2449 100644
--- a/net/netfilter/xt_limit.c
+++ b/net/netfilter/xt_limit.c
@@ -46,7 +46,7 @@ MODULE_ALIAS("ip6t_limit");
 
    See Alexey's formal explanation in net/sched/sch_tbf.c.
 
-   To get the maxmum range, we multiply by this factor (ie. you get N
+   To get the maximum range, we multiply by this factor (ie. you get N
    credits per jiffy).  We want to allow a rate as low as 1 per day
    (slowest userspace tool allows), which means
    CREDITS_PER_JIFFY*HZ*60*60*24 < 2^32. ie. */
-- 
2.11.0

^ permalink raw reply related

* [PATCH 20/47] netfilter: x_tables: fix build with CONFIG_COMPAT=n
From: Pablo Neira Ayuso @ 2018-03-30 11:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Florian Westphal <fw@strlen.de>

I placed the helpers within CONFIG_COMPAT section, move them
outside.

Fixes: 472ebdcd15ebdb ("netfilter: x_tables: check error target size too")
Fixes: 07a9da51b4b6ae ("netfilter: x_tables: check standard verdicts in core")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/x_tables.c | 62 ++++++++++++++++++++++++------------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 7521e8a72c06..bac932f1c582 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -577,6 +577,37 @@ int xt_check_table_hooks(const struct xt_table_info *info, unsigned int valid_ho
 }
 EXPORT_SYMBOL(xt_check_table_hooks);
 
+static bool verdict_ok(int verdict)
+{
+	if (verdict > 0)
+		return true;
+
+	if (verdict < 0) {
+		int v = -verdict - 1;
+
+		if (verdict == XT_RETURN)
+			return true;
+
+		switch (v) {
+		case NF_ACCEPT: return true;
+		case NF_DROP: return true;
+		case NF_QUEUE: return true;
+		default:
+			break;
+		}
+
+		return false;
+	}
+
+	return false;
+}
+
+static bool error_tg_ok(unsigned int usersize, unsigned int kernsize,
+			const char *msg, unsigned int msglen)
+{
+	return usersize == kernsize && strnlen(msg, msglen) < msglen;
+}
+
 #ifdef CONFIG_COMPAT
 int xt_compat_add_offset(u_int8_t af, unsigned int offset, int delta)
 {
@@ -736,37 +767,6 @@ struct compat_xt_error_target {
 	char errorname[XT_FUNCTION_MAXNAMELEN];
 };
 
-static bool verdict_ok(int verdict)
-{
-	if (verdict > 0)
-		return true;
-
-	if (verdict < 0) {
-		int v = -verdict - 1;
-
-		if (verdict == XT_RETURN)
-			return true;
-
-		switch (v) {
-		case NF_ACCEPT: return true;
-		case NF_DROP: return true;
-		case NF_QUEUE: return true;
-		default:
-			break;
-		}
-
-		return false;
-	}
-
-	return false;
-}
-
-static bool error_tg_ok(unsigned int usersize, unsigned int kernsize,
-			const char *msg, unsigned int msglen)
-{
-	return usersize == kernsize && strnlen(msg, msglen) < msglen;
-}
-
 int xt_compat_check_entry_offsets(const void *base, const char *elems,
 				  unsigned int target_offset,
 				  unsigned int next_offset)
-- 
2.11.0

^ permalink raw reply related

* [PATCH 21/47] ipvs: use true and false for boolean values
From: Pablo Neira Ayuso @ 2018-03-30 11:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330114334.18664-1-pablo@netfilter.org>

From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>

Assign true or false to boolean variables instead of an integer value.

This issue was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/ipvs/ip_vs_lblc.c  | 4 ++--
 net/netfilter/ipvs/ip_vs_lblcr.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index 6a340c94c4b8..942e835caf7f 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -238,7 +238,7 @@ static void ip_vs_lblc_flush(struct ip_vs_service *svc)
 	int i;
 
 	spin_lock_bh(&svc->sched_lock);
-	tbl->dead = 1;
+	tbl->dead = true;
 	for (i = 0; i < IP_VS_LBLC_TAB_SIZE; i++) {
 		hlist_for_each_entry_safe(en, next, &tbl->bucket[i], list) {
 			ip_vs_lblc_del(en);
@@ -369,7 +369,7 @@ static int ip_vs_lblc_init_svc(struct ip_vs_service *svc)
 	tbl->max_size = IP_VS_LBLC_TAB_SIZE*16;
 	tbl->rover = 0;
 	tbl->counter = 1;
-	tbl->dead = 0;
+	tbl->dead = false;
 	tbl->svc = svc;
 
 	/*
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index 0627881128da..a5acab25c36b 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -404,7 +404,7 @@ static void ip_vs_lblcr_flush(struct ip_vs_service *svc)
 	struct hlist_node *next;
 
 	spin_lock_bh(&svc->sched_lock);
-	tbl->dead = 1;
+	tbl->dead = true;
 	for (i = 0; i < IP_VS_LBLCR_TAB_SIZE; i++) {
 		hlist_for_each_entry_safe(en, next, &tbl->bucket[i], list) {
 			ip_vs_lblcr_free(en);
@@ -532,7 +532,7 @@ static int ip_vs_lblcr_init_svc(struct ip_vs_service *svc)
 	tbl->max_size = IP_VS_LBLCR_TAB_SIZE*16;
 	tbl->rover = 0;
 	tbl->counter = 1;
-	tbl->dead = 0;
+	tbl->dead = false;
 	tbl->svc = svc;
 
 	/*
-- 
2.11.0

^ permalink raw reply related

* [PATCH 22/47] netfilter: Refactor nf_conncount
From: Pablo Neira Ayuso @ 2018-03-30 11:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330114334.18664-1-pablo@netfilter.org>

From: Yi-Hung Wei <yihung.wei@gmail.com>

Remove parameter 'family' in nf_conncount_count() and count_tree().
It is because the parameter is not useful after commit 625c556118f3
("netfilter: connlimit: split xt_connlimit into front and backend").

Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
Acked-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_conntrack_count.h | 1 -
 net/netfilter/nf_conncount.c               | 4 +---
 net/netfilter/xt_connlimit.c               | 4 ++--
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_count.h b/include/net/netfilter/nf_conntrack_count.h
index adf8db44cf86..e61184fbfb71 100644
--- a/include/net/netfilter/nf_conntrack_count.h
+++ b/include/net/netfilter/nf_conntrack_count.h
@@ -11,7 +11,6 @@ void nf_conncount_destroy(struct net *net, unsigned int family,
 unsigned int nf_conncount_count(struct net *net,
 				struct nf_conncount_data *data,
 				const u32 *key,
-				unsigned int family,
 				const struct nf_conntrack_tuple *tuple,
 				const struct nf_conntrack_zone *zone);
 #endif
diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index 6d65389e308f..9305a08b4422 100644
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -158,7 +158,6 @@ static void tree_nodes_free(struct rb_root *root,
 static unsigned int
 count_tree(struct net *net, struct rb_root *root,
 	   const u32 *key, u8 keylen,
-	   u8 family,
 	   const struct nf_conntrack_tuple *tuple,
 	   const struct nf_conntrack_zone *zone)
 {
@@ -246,7 +245,6 @@ count_tree(struct net *net, struct rb_root *root,
 unsigned int nf_conncount_count(struct net *net,
 				struct nf_conncount_data *data,
 				const u32 *key,
-				unsigned int family,
 				const struct nf_conntrack_tuple *tuple,
 				const struct nf_conntrack_zone *zone)
 {
@@ -259,7 +257,7 @@ unsigned int nf_conncount_count(struct net *net,
 
 	spin_lock_bh(&nf_conncount_locks[hash % CONNCOUNT_LOCK_SLOTS]);
 
-	count = count_tree(net, root, key, data->keylen, family, tuple, zone);
+	count = count_tree(net, root, key, data->keylen, tuple, zone);
 
 	spin_unlock_bh(&nf_conncount_locks[hash % CONNCOUNT_LOCK_SLOTS]);
 
diff --git a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
index b1b17b9353e1..6275106ccf50 100644
--- a/net/netfilter/xt_connlimit.c
+++ b/net/netfilter/xt_connlimit.c
@@ -67,8 +67,8 @@ connlimit_mt(const struct sk_buff *skb, struct xt_action_param *par)
 		key[1] = zone->id;
 	}
 
-	connections = nf_conncount_count(net, info->data, key,
-					 xt_family(par), tuple_ptr, zone);
+	connections = nf_conncount_count(net, info->data, key, tuple_ptr,
+					 zone);
 	if (connections == 0)
 		/* kmalloc failed, drop it entirely */
 		goto hotdrop;
-- 
2.11.0

^ permalink raw reply related

* [PATCH 23/47] netfilter: conncount: Support count only use case
From: Pablo Neira Ayuso @ 2018-03-30 11:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330114334.18664-1-pablo@netfilter.org>

From: Yi-Hung Wei <yihung.wei@gmail.com>

Currently, nf_conncount_count() counts the number of connections that
matches key and inserts a conntrack 'tuple' with the same key into the
accounting data structure.  This patch supports another use case that only
counts the number of connections where 'tuple' is not provided.  Therefore,
proper changes are made on nf_conncount_count() to support the case where
'tuple' is NULL.  This could be useful for querying statistics or
debugging purpose.

Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
Acked-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conncount.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index 9305a08b4422..153e690e2893 100644
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -104,7 +104,7 @@ static unsigned int check_hlist(struct net *net,
 	struct nf_conn *found_ct;
 	unsigned int length = 0;
 
-	*addit = true;
+	*addit = tuple ? true : false;
 
 	/* check the saved connections */
 	hlist_for_each_entry_safe(conn, n, head, node) {
@@ -117,7 +117,7 @@ static unsigned int check_hlist(struct net *net,
 
 		found_ct = nf_ct_tuplehash_to_ctrack(found);
 
-		if (nf_ct_tuple_equal(&conn->tuple, tuple)) {
+		if (tuple && nf_ct_tuple_equal(&conn->tuple, tuple)) {
 			/*
 			 * Just to be sure we have it only once in the list.
 			 * We should not see tuples twice unless someone hooks
@@ -220,6 +220,9 @@ count_tree(struct net *net, struct rb_root *root,
 		goto restart;
 	}
 
+	if (!tuple)
+		return 0;
+
 	/* no match, need to insert new node */
 	rbconn = kmem_cache_alloc(conncount_rb_cachep, GFP_ATOMIC);
 	if (rbconn == NULL)
@@ -242,6 +245,9 @@ count_tree(struct net *net, struct rb_root *root,
 	return 1;
 }
 
+/* Count and return number of conntrack entries in 'net' with particular 'key'.
+ * If 'tuple' is not null, insert it into the accounting data structure.
+ */
 unsigned int nf_conncount_count(struct net *net,
 				struct nf_conncount_data *data,
 				const u32 *key,
-- 
2.11.0

^ permalink raw reply related

* [PATCH 24/47] netfilter: nft_ct: add NFT_CT_{SRC,DST}_{IP,IP6}
From: Pablo Neira Ayuso @ 2018-03-30 11:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330114334.18664-1-pablo@netfilter.org>

All existing keys, except the NFT_CT_SRC and NFT_CT_DST are assumed to
have strict datatypes. This is causing problems with sets and
concatenations given the specific length of these keys is not known.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Acked-by: Florian Westphal <fw@strlen.de>
---
 include/uapi/linux/netfilter/nf_tables.h | 12 ++++++++--
 net/netfilter/nft_ct.c                   | 38 ++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 66dceee0ae30..6a3d653d5b27 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -909,8 +909,8 @@ enum nft_rt_attributes {
  * @NFT_CT_EXPIRATION: relative conntrack expiration time in ms
  * @NFT_CT_HELPER: connection tracking helper assigned to conntrack
  * @NFT_CT_L3PROTOCOL: conntrack layer 3 protocol
- * @NFT_CT_SRC: conntrack layer 3 protocol source (IPv4/IPv6 address)
- * @NFT_CT_DST: conntrack layer 3 protocol destination (IPv4/IPv6 address)
+ * @NFT_CT_SRC: conntrack layer 3 protocol source (IPv4/IPv6 address, deprecated)
+ * @NFT_CT_DST: conntrack layer 3 protocol destination (IPv4/IPv6 address, deprecated)
  * @NFT_CT_PROTOCOL: conntrack layer 4 protocol
  * @NFT_CT_PROTO_SRC: conntrack layer 4 protocol source
  * @NFT_CT_PROTO_DST: conntrack layer 4 protocol destination
@@ -920,6 +920,10 @@ enum nft_rt_attributes {
  * @NFT_CT_AVGPKT: conntrack average bytes per packet
  * @NFT_CT_ZONE: conntrack zone
  * @NFT_CT_EVENTMASK: ctnetlink events to be generated for this conntrack
+ * @NFT_CT_SRC_IP: conntrack layer 3 protocol source (IPv4 address)
+ * @NFT_CT_DST_IP: conntrack layer 3 protocol destination (IPv4 address)
+ * @NFT_CT_SRC_IP6: conntrack layer 3 protocol source (IPv6 address)
+ * @NFT_CT_DST_IP6: conntrack layer 3 protocol destination (IPv6 address)
  */
 enum nft_ct_keys {
 	NFT_CT_STATE,
@@ -941,6 +945,10 @@ enum nft_ct_keys {
 	NFT_CT_AVGPKT,
 	NFT_CT_ZONE,
 	NFT_CT_EVENTMASK,
+	NFT_CT_SRC_IP,
+	NFT_CT_DST_IP,
+	NFT_CT_SRC_IP6,
+	NFT_CT_DST_IP6,
 };
 
 /**
diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index 6ab274b14484..ea737fd789e8 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -196,6 +196,26 @@ static void nft_ct_get_eval(const struct nft_expr *expr,
 	case NFT_CT_PROTO_DST:
 		nft_reg_store16(dest, (__force u16)tuple->dst.u.all);
 		return;
+	case NFT_CT_SRC_IP:
+		if (nf_ct_l3num(ct) != NFPROTO_IPV4)
+			goto err;
+		*dest = tuple->src.u3.ip;
+		return;
+	case NFT_CT_DST_IP:
+		if (nf_ct_l3num(ct) != NFPROTO_IPV4)
+			goto err;
+		*dest = tuple->dst.u3.ip;
+		return;
+	case NFT_CT_SRC_IP6:
+		if (nf_ct_l3num(ct) != NFPROTO_IPV6)
+			goto err;
+		memcpy(dest, tuple->src.u3.ip6, sizeof(struct in6_addr));
+		return;
+	case NFT_CT_DST_IP6:
+		if (nf_ct_l3num(ct) != NFPROTO_IPV6)
+			goto err;
+		memcpy(dest, tuple->dst.u3.ip6, sizeof(struct in6_addr));
+		return;
 	default:
 		break;
 	}
@@ -419,6 +439,20 @@ static int nft_ct_get_init(const struct nft_ctx *ctx,
 			return -EAFNOSUPPORT;
 		}
 		break;
+	case NFT_CT_SRC_IP:
+	case NFT_CT_DST_IP:
+		if (tb[NFTA_CT_DIRECTION] == NULL)
+			return -EINVAL;
+
+		len = FIELD_SIZEOF(struct nf_conntrack_tuple, src.u3.ip);
+		break;
+	case NFT_CT_SRC_IP6:
+	case NFT_CT_DST_IP6:
+		if (tb[NFTA_CT_DIRECTION] == NULL)
+			return -EINVAL;
+
+		len = FIELD_SIZEOF(struct nf_conntrack_tuple, src.u3.ip6);
+		break;
 	case NFT_CT_PROTO_SRC:
 	case NFT_CT_PROTO_DST:
 		if (tb[NFTA_CT_DIRECTION] == NULL)
@@ -588,6 +622,10 @@ static int nft_ct_get_dump(struct sk_buff *skb, const struct nft_expr *expr)
 	switch (priv->key) {
 	case NFT_CT_SRC:
 	case NFT_CT_DST:
+	case NFT_CT_SRC_IP:
+	case NFT_CT_DST_IP:
+	case NFT_CT_SRC_IP6:
+	case NFT_CT_DST_IP6:
 	case NFT_CT_PROTO_SRC:
 	case NFT_CT_PROTO_DST:
 		if (nla_put_u8(skb, NFTA_CT_DIRECTION, priv->dir))
-- 
2.11.0

^ permalink raw reply related

* [PATCH 25/47] netfilter: cttimeout: remove VLA usage
From: Pablo Neira Ayuso @ 2018-03-30 11:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330114334.18664-1-pablo@netfilter.org>

From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>

In preparation to enabling -Wvla, remove VLA and replace it
with dynamic memory allocation.

>From a security viewpoint, the use of Variable Length Arrays can be
a vector for stack overflow attacks. Also, in general, as the code
evolves it is easy to lose track of how big a VLA can get. Thus, we
can end up having segfaults that are hard to debug.

Also, fixed as part of the directive to remove all VLAs from
the kernel: https://lkml.org/lkml/2018/3/7/621

While at it, remove likely() notation which is not necessary from the
control plane code.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink_cttimeout.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/nfnetlink_cttimeout.c b/net/netfilter/nfnetlink_cttimeout.c
index 95b04702a655..9ee5fa551fa6 100644
--- a/net/netfilter/nfnetlink_cttimeout.c
+++ b/net/netfilter/nfnetlink_cttimeout.c
@@ -51,19 +51,27 @@ ctnl_timeout_parse_policy(void *timeouts,
 			  const struct nf_conntrack_l4proto *l4proto,
 			  struct net *net, const struct nlattr *attr)
 {
+	struct nlattr **tb;
 	int ret = 0;
 
-	if (likely(l4proto->ctnl_timeout.nlattr_to_obj)) {
-		struct nlattr *tb[l4proto->ctnl_timeout.nlattr_max+1];
+	if (!l4proto->ctnl_timeout.nlattr_to_obj)
+		return 0;
 
-		ret = nla_parse_nested(tb, l4proto->ctnl_timeout.nlattr_max,
-				       attr, l4proto->ctnl_timeout.nla_policy,
-				       NULL);
-		if (ret < 0)
-			return ret;
+	tb = kcalloc(l4proto->ctnl_timeout.nlattr_max + 1, sizeof(*tb),
+		     GFP_KERNEL);
 
-		ret = l4proto->ctnl_timeout.nlattr_to_obj(tb, net, timeouts);
-	}
+	if (!tb)
+		return -ENOMEM;
+
+	ret = nla_parse_nested(tb, l4proto->ctnl_timeout.nlattr_max, attr,
+			       l4proto->ctnl_timeout.nla_policy, NULL);
+	if (ret < 0)
+		goto err;
+
+	ret = l4proto->ctnl_timeout.nlattr_to_obj(tb, net, timeouts);
+
+err:
+	kfree(tb);
 	return ret;
 }
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH 27/47] netfilter: nf_tables: remove VLA usage
From: Pablo Neira Ayuso @ 2018-03-30 11:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330114334.18664-1-pablo@netfilter.org>

From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>

In preparation to enabling -Wvla, remove VLA and replace it
with dynamic memory allocation.

>From a security viewpoint, the use of Variable Length Arrays can be
a vector for stack overflow attacks. Also, in general, as the code
evolves it is easy to lose track of how big a VLA can get. Thus, we
can end up having segfaults that are hard to debug.

Also, fixed as part of the directive to remove all VLAs from
the kernel: https://lkml.org/lkml/2018/3/7/621

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 8cc7fc970f0c..92f5606b0dea 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -4357,16 +4357,20 @@ static struct nft_object *nft_obj_init(const struct nft_ctx *ctx,
 				       const struct nft_object_type *type,
 				       const struct nlattr *attr)
 {
-	struct nlattr *tb[type->maxattr + 1];
+	struct nlattr **tb;
 	const struct nft_object_ops *ops;
 	struct nft_object *obj;
-	int err;
+	int err = -ENOMEM;
+
+	tb = kmalloc_array(type->maxattr + 1, sizeof(*tb), GFP_KERNEL);
+	if (!tb)
+		goto err1;
 
 	if (attr) {
 		err = nla_parse_nested(tb, type->maxattr, attr, type->policy,
 				       NULL);
 		if (err < 0)
-			goto err1;
+			goto err2;
 	} else {
 		memset(tb, 0, sizeof(tb[0]) * (type->maxattr + 1));
 	}
@@ -4375,7 +4379,7 @@ static struct nft_object *nft_obj_init(const struct nft_ctx *ctx,
 		ops = type->select_ops(ctx, (const struct nlattr * const *)tb);
 		if (IS_ERR(ops)) {
 			err = PTR_ERR(ops);
-			goto err1;
+			goto err2;
 		}
 	} else {
 		ops = type->ops;
@@ -4383,18 +4387,21 @@ static struct nft_object *nft_obj_init(const struct nft_ctx *ctx,
 
 	err = -ENOMEM;
 	obj = kzalloc(sizeof(*obj) + ops->size, GFP_KERNEL);
-	if (obj == NULL)
-		goto err1;
+	if (!obj)
+		goto err2;
 
 	err = ops->init(ctx, (const struct nlattr * const *)tb, obj);
 	if (err < 0)
-		goto err2;
+		goto err3;
 
 	obj->ops = ops;
 
+	kfree(tb);
 	return obj;
-err2:
+err3:
 	kfree(obj);
+err2:
+	kfree(tb);
 err1:
 	return ERR_PTR(err);
 }
-- 
2.11.0

^ permalink raw reply related

* [PATCH 26/47] netfilter: nfnetlink_cthelper: Remove VLA usage
From: Pablo Neira Ayuso @ 2018-03-30 11:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330114334.18664-1-pablo@netfilter.org>

From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>

In preparation to enabling -Wvla, remove VLA and replace it
with dynamic memory allocation.

>From a security viewpoint, the use of Variable Length Arrays can be
a vector for stack overflow attacks. Also, in general, as the code
evolves it is easy to lose track of how big a VLA can get. Thus, we
can end up having segfaults that are hard to debug.

Also, fixed as part of the directive to remove all VLAs from
the kernel: https://lkml.org/lkml/2018/3/7/621

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink_cthelper.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/nfnetlink_cthelper.c b/net/netfilter/nfnetlink_cthelper.c
index d33ce6d5ebce..4a4b293fb2e5 100644
--- a/net/netfilter/nfnetlink_cthelper.c
+++ b/net/netfilter/nfnetlink_cthelper.c
@@ -314,23 +314,30 @@ nfnl_cthelper_update_policy_one(const struct nf_conntrack_expect_policy *policy,
 static int nfnl_cthelper_update_policy_all(struct nlattr *tb[],
 					   struct nf_conntrack_helper *helper)
 {
-	struct nf_conntrack_expect_policy new_policy[helper->expect_class_max + 1];
+	struct nf_conntrack_expect_policy *new_policy;
 	struct nf_conntrack_expect_policy *policy;
-	int i, err;
+	int i, ret = 0;
+
+	new_policy = kmalloc_array(helper->expect_class_max + 1,
+				   sizeof(*new_policy), GFP_KERNEL);
+	if (!new_policy)
+		return -ENOMEM;
 
 	/* Check first that all policy attributes are well-formed, so we don't
 	 * leave things in inconsistent state on errors.
 	 */
 	for (i = 0; i < helper->expect_class_max + 1; i++) {
 
-		if (!tb[NFCTH_POLICY_SET + i])
-			return -EINVAL;
+		if (!tb[NFCTH_POLICY_SET + i]) {
+			ret = -EINVAL;
+			goto err;
+		}
 
-		err = nfnl_cthelper_update_policy_one(&helper->expect_policy[i],
+		ret = nfnl_cthelper_update_policy_one(&helper->expect_policy[i],
 						      &new_policy[i],
 						      tb[NFCTH_POLICY_SET + i]);
-		if (err < 0)
-			return err;
+		if (ret < 0)
+			goto err;
 	}
 	/* Now we can safely update them. */
 	for (i = 0; i < helper->expect_class_max + 1; i++) {
@@ -340,7 +347,9 @@ static int nfnl_cthelper_update_policy_all(struct nlattr *tb[],
 		policy->timeout	= new_policy->timeout;
 	}
 
-	return 0;
+err:
+	kfree(new_policy);
+	return ret;
 }
 
 static int nfnl_cthelper_update_policy(struct nf_conntrack_helper *helper,
-- 
2.11.0

^ permalink raw reply related

* [PATCH 28/47] netfilter: ebtables: use ADD_COUNTER macro
From: Pablo Neira Ayuso @ 2018-03-30 11:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330114334.18664-1-pablo@netfilter.org>

From: Taehee Yoo <ap420073@gmail.com>

xtables uses ADD_COUNTER macro to increase
packet and byte count. ebtables also can use this.

Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/bridge/netfilter/ebtables.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 217aa79f7b2a..9a26d2b7420f 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -223,9 +223,7 @@ unsigned int ebt_do_table(struct sk_buff *skb,
 			return NF_DROP;
 		}
 
-		/* increase counter */
-		(*(counter_base + i)).pcnt++;
-		(*(counter_base + i)).bcnt += skb->len;
+		ADD_COUNTER(*(counter_base + i), 1, skb->len);
 
 		/* these should only watch: not modify, nor tell us
 		 * what to do with the packet
@@ -968,10 +966,9 @@ static void get_counters(const struct ebt_counter *oldcounters,
 		if (cpu == 0)
 			continue;
 		counter_base = COUNTER_BASE(oldcounters, nentries, cpu);
-		for (i = 0; i < nentries; i++) {
-			counters[i].pcnt += counter_base[i].pcnt;
-			counters[i].bcnt += counter_base[i].bcnt;
-		}
+		for (i = 0; i < nentries; i++)
+			ADD_COUNTER(counters[i], counter_base[i].pcnt,
+				    counter_base[i].bcnt);
 	}
 }
 
@@ -1324,10 +1321,8 @@ static int do_update_counters(struct net *net, const char *name,
 	write_lock_bh(&t->lock);
 
 	/* we add to the counters of the first cpu */
-	for (i = 0; i < num_counters; i++) {
-		t->private->counters[i].pcnt += tmp[i].pcnt;
-		t->private->counters[i].bcnt += tmp[i].bcnt;
-	}
+	for (i = 0; i < num_counters; i++)
+		ADD_COUNTER(t->private->counters[i], tmp[i].pcnt, tmp[i].bcnt);
 
 	write_unlock_bh(&t->lock);
 	ret = 0;
-- 
2.11.0

^ permalink raw reply related

* [PATCH 29/47] netfilter: xt_conntrack: Support bit-shifting for CONNMARK & MARK targets.
From: Pablo Neira Ayuso @ 2018-03-30 11:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180330114334.18664-1-pablo@netfilter.org>

From: Jack Ma <jack.ma@alliedtelesis.co.nz>

This patch introduces a new feature that allows bitshifting (left
and right) operations to co-operate with existing iptables options.

Reviewed-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Jack Ma <jack.ma@alliedtelesis.co.nz>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/uapi/linux/netfilter/xt_connmark.h | 10 ++++
 net/netfilter/xt_connmark.c                | 77 +++++++++++++++++++++++-------
 2 files changed, 70 insertions(+), 17 deletions(-)

diff --git a/include/uapi/linux/netfilter/xt_connmark.h b/include/uapi/linux/netfilter/xt_connmark.h
index 408a9654f05c..1aa5c955ee1e 100644
--- a/include/uapi/linux/netfilter/xt_connmark.h
+++ b/include/uapi/linux/netfilter/xt_connmark.h
@@ -19,11 +19,21 @@ enum {
 	XT_CONNMARK_RESTORE
 };
 
+enum {
+	D_SHIFT_LEFT = 0,
+	D_SHIFT_RIGHT,
+};
+
 struct xt_connmark_tginfo1 {
 	__u32 ctmark, ctmask, nfmask;
 	__u8 mode;
 };
 
+struct xt_connmark_tginfo2 {
+	__u32 ctmark, ctmask, nfmask;
+	__u8 shift_dir, shift_bits, mode;
+};
+
 struct xt_connmark_mtinfo1 {
 	__u32 mark, mask;
 	__u8 invert;
diff --git a/net/netfilter/xt_connmark.c b/net/netfilter/xt_connmark.c
index 809639ce6f5a..773da82190dc 100644
--- a/net/netfilter/xt_connmark.c
+++ b/net/netfilter/xt_connmark.c
@@ -36,9 +36,10 @@ MODULE_ALIAS("ipt_connmark");
 MODULE_ALIAS("ip6t_connmark");
 
 static unsigned int
-connmark_tg(struct sk_buff *skb, const struct xt_action_param *par)
+connmark_tg_shift(struct sk_buff *skb,
+		const struct xt_connmark_tginfo1 *info,
+		u8 shift_bits, u8 shift_dir)
 {
-	const struct xt_connmark_tginfo1 *info = par->targinfo;
 	enum ip_conntrack_info ctinfo;
 	struct nf_conn *ct;
 	u_int32_t newmark;
@@ -50,6 +51,10 @@ connmark_tg(struct sk_buff *skb, const struct xt_action_param *par)
 	switch (info->mode) {
 	case XT_CONNMARK_SET:
 		newmark = (ct->mark & ~info->ctmask) ^ info->ctmark;
+		if (shift_dir == D_SHIFT_RIGHT)
+			newmark >>= shift_bits;
+		else
+			newmark <<= shift_bits;
 		if (ct->mark != newmark) {
 			ct->mark = newmark;
 			nf_conntrack_event_cache(IPCT_MARK, ct);
@@ -57,7 +62,11 @@ connmark_tg(struct sk_buff *skb, const struct xt_action_param *par)
 		break;
 	case XT_CONNMARK_SAVE:
 		newmark = (ct->mark & ~info->ctmask) ^
-		          (skb->mark & info->nfmask);
+			  (skb->mark & info->nfmask);
+		if (shift_dir == D_SHIFT_RIGHT)
+			newmark >>= shift_bits;
+		else
+			newmark <<= shift_bits;
 		if (ct->mark != newmark) {
 			ct->mark = newmark;
 			nf_conntrack_event_cache(IPCT_MARK, ct);
@@ -65,14 +74,34 @@ connmark_tg(struct sk_buff *skb, const struct xt_action_param *par)
 		break;
 	case XT_CONNMARK_RESTORE:
 		newmark = (skb->mark & ~info->nfmask) ^
-		          (ct->mark & info->ctmask);
+			  (ct->mark & info->ctmask);
+		if (shift_dir == D_SHIFT_RIGHT)
+			newmark >>= shift_bits;
+		else
+			newmark <<= shift_bits;
 		skb->mark = newmark;
 		break;
 	}
-
 	return XT_CONTINUE;
 }
 
+static unsigned int
+connmark_tg(struct sk_buff *skb, const struct xt_action_param *par)
+{
+	const struct xt_connmark_tginfo1 *info = par->targinfo;
+
+	return connmark_tg_shift(skb, info, 0, 0);
+}
+
+static unsigned int
+connmark_tg_v2(struct sk_buff *skb, const struct xt_action_param *par)
+{
+	const struct xt_connmark_tginfo2 *info = par->targinfo;
+
+	return connmark_tg_shift(skb, (const struct xt_connmark_tginfo1 *)info,
+				 info->shift_bits, info->shift_dir);
+}
+
 static int connmark_tg_check(const struct xt_tgchk_param *par)
 {
 	int ret;
@@ -119,15 +148,27 @@ static void connmark_mt_destroy(const struct xt_mtdtor_param *par)
 	nf_ct_netns_put(par->net, par->family);
 }
 
-static struct xt_target connmark_tg_reg __read_mostly = {
-	.name           = "CONNMARK",
-	.revision       = 1,
-	.family         = NFPROTO_UNSPEC,
-	.checkentry     = connmark_tg_check,
-	.target         = connmark_tg,
-	.targetsize     = sizeof(struct xt_connmark_tginfo1),
-	.destroy        = connmark_tg_destroy,
-	.me             = THIS_MODULE,
+static struct xt_target connmark_tg_reg[] __read_mostly = {
+	{
+		.name           = "CONNMARK",
+		.revision       = 1,
+		.family         = NFPROTO_UNSPEC,
+		.checkentry     = connmark_tg_check,
+		.target         = connmark_tg,
+		.targetsize     = sizeof(struct xt_connmark_tginfo1),
+		.destroy        = connmark_tg_destroy,
+		.me             = THIS_MODULE,
+	},
+	{
+		.name           = "CONNMARK",
+		.revision       = 2,
+		.family         = NFPROTO_UNSPEC,
+		.checkentry     = connmark_tg_check,
+		.target         = connmark_tg_v2,
+		.targetsize     = sizeof(struct xt_connmark_tginfo2),
+		.destroy        = connmark_tg_destroy,
+		.me             = THIS_MODULE,
+	}
 };
 
 static struct xt_match connmark_mt_reg __read_mostly = {
@@ -145,12 +186,14 @@ static int __init connmark_mt_init(void)
 {
 	int ret;
 
-	ret = xt_register_target(&connmark_tg_reg);
+	ret = xt_register_targets(connmark_tg_reg,
+				  ARRAY_SIZE(connmark_tg_reg));
 	if (ret < 0)
 		return ret;
 	ret = xt_register_match(&connmark_mt_reg);
 	if (ret < 0) {
-		xt_unregister_target(&connmark_tg_reg);
+		xt_unregister_targets(connmark_tg_reg,
+				      ARRAY_SIZE(connmark_tg_reg));
 		return ret;
 	}
 	return 0;
@@ -159,7 +202,7 @@ static int __init connmark_mt_init(void)
 static void __exit connmark_mt_exit(void)
 {
 	xt_unregister_match(&connmark_mt_reg);
-	xt_unregister_target(&connmark_tg_reg);
+	xt_unregister_target(connmark_tg_reg);
 }
 
 module_init(connmark_mt_init);
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH net-next 4/6] inet: frags: use rhashtables for reassembly units
From: Kirill Tkhai @ 2018-03-30 11:44 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller
  Cc: netdev, Florian Westphal, Herbert Xu, Thomas Graf,
	Jesper Dangaard Brouer, Alexander Aring, Stefan Schmidt,
	Eric Dumazet, Nikolay Aleksandrov
In-Reply-To: <20180330052241.206667-5-edumazet@google.com>

Hi, Eric,

On 30.03.2018 08:22, Eric Dumazet wrote:
> Some applications still rely on IP fragmentation, and to be fair linux
> reassembly unit is not working under any serious load.
> 
> It uses static hash tables of 1024 buckets, and up to 128 items per bucket (!!!)
> 
> A work queue is supposed to garbage collect items when host is under memory
> pressure, and doing a hash rebuild, changing seed used in hash computations.
> 
> This work queue blocks softirqs for up to 25 ms when doing a hash rebuild,
> occurring every 5 seconds if host is under fire.
> 
> Then there is the problem of sharing this hash table for all netns.
> 
> It is time to switch to rhashtables, and allocate one of them per netns
> to speedup netns dismantle, since this is a critical metric these days.
> 
> Lookup is now using RCU. A followup patch will even remove
> the refcount hold/release left from prior implementation and save
> a couple of atomic operations.
> 
> Before this patch, 16 cpus (16 RX queue NIC) could not handle more
> than 1 Mpps frags DDOS.
> 
> After the patch, I reach 7 Mpps without any tuning, and can use up to 2GB
> of storage for the fragments.

Great results!

Please, see some comments below.
 
> $ grep FRAG /proc/net/sockstat
> FRAG: inuse 1966916 memory 2140004608
> 
> A followup patch will change the limits for 64bit arches.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Florian Westphal <fw@strlen.de>
> Cc: Nikolay Aleksandrov <nikolay@redhat.com>
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> Cc: Alexander Aring <alex.aring@gmail.com>
> Cc: Stefan Schmidt <stefan@osg.samsung.com>
> ---
>  Documentation/networking/ip-sysctl.txt  |   7 +-
>  include/net/inet_frag.h                 |  99 +++---
>  include/net/ipv6.h                      |  20 +-
>  net/ieee802154/6lowpan/6lowpan_i.h      |  26 +-
>  net/ieee802154/6lowpan/reassembly.c     | 108 +++----
>  net/ipv4/inet_fragment.c                | 399 +++++-------------------
>  net/ipv4/ip_fragment.c                  | 165 +++++-----
>  net/ipv6/netfilter/nf_conntrack_reasm.c |  62 ++--
>  net/ipv6/reassembly.c                   | 152 +++++----
>  9 files changed, 344 insertions(+), 694 deletions(-)
> 
> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> index 1d1120753ae82d0aee3e934a3d9c074b70dcbca6..c3b65f24e58aa72b720861d816fb76f9956800f0 100644
> --- a/Documentation/networking/ip-sysctl.txt
> +++ b/Documentation/networking/ip-sysctl.txt
> @@ -134,13 +134,10 @@ min_adv_mss - INTEGER
>  IP Fragmentation:
>  
>  ipfrag_high_thresh - INTEGER
> -	Maximum memory used to reassemble IP fragments. When
> -	ipfrag_high_thresh bytes of memory is allocated for this purpose,
> -	the fragment handler will toss packets until ipfrag_low_thresh
> -	is reached. This also serves as a maximum limit to namespaces
> -	different from the initial one.
> +	Maximum memory used to reassemble IP fragments.
>  
>  ipfrag_low_thresh - INTEGER
> +	(Obsolete since linux-4.17)
>  	Maximum memory used to reassemble IP fragments before the kernel
>  	begins to remove incomplete fragment queues to free up resources.
>  	The kernel still accepts new fragments for defragmentation.
> diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
> index 69e531ed81894393e07cac9e953825fcb55ef42a..05099f9f980e2384c0c8cd7e74659656b585cd22 100644
> --- a/include/net/inet_frag.h
> +++ b/include/net/inet_frag.h
> @@ -2,15 +2,20 @@
>  #ifndef __NET_FRAG_H__
>  #define __NET_FRAG_H__
>  
> +#include <linux/rhashtable.h>
> +
>  struct netns_frags {
> -	/* Keep atomic mem on separate cachelines in structs that include it */
> -	atomic_t		mem ____cacheline_aligned_in_smp;
>  	/* sysctls */
>  	int			timeout;
>  	int			high_thresh;
>  	int			low_thresh;
>  	int			max_dist;
>  	struct inet_frags	*f;
> +
> +	/* Keep atomic mem on separate cachelines in structs that include it */
> +	atomic_t		mem ____cacheline_aligned_in_smp;

The patch is big, and it seems it's possible to extract refactorings like this
in separate patch/patches. Here just two lines moved down.

> +
> +	struct rhashtable       rhashtable ____cacheline_aligned_in_smp;
>  };
>  
>  /**
> @@ -26,12 +31,31 @@ enum {
>  	INET_FRAG_COMPLETE	= BIT(2),
>  };
>  
> +struct frag_v4_compare_key {
> +	__be32		saddr;
> +	__be32		daddr;
> +	u32		user;
> +	u32		vif;
> +	__be16		id;
> +	u16		protocol;
> +};
> +
> +struct frag_v6_compare_key {
> +	struct in6_addr	saddr;
> +	struct in6_addr	daddr;
> +	u32		user;
> +	__be32		id;
> +	u32		iif;
> +};
> +
>  /**
>   * struct inet_frag_queue - fragment queue
>   *
> - * @lock: spinlock protecting the queue
> + * @node: rhash node
> + * @key: keys identifying this frag.
>   * @timer: queue expiration timer
> - * @list: hash bucket list
> + * @net: namespace that this frag belongs to
> + * @lock: spinlock protecting this frag
>   * @refcnt: reference count of the queue
>   * @fragments: received fragments head
>   * @fragments_tail: received fragments tail
> @@ -40,66 +64,38 @@ enum {
>   * @meat: length of received fragments so far
>   * @flags: fragment queue flags
>   * @max_size: maximum received fragment size
> - * @net: namespace that this frag belongs to
> - * @list_evictor: list of queues to forcefully evict (e.g. due to low memory)
> + * @rcu: rcu head for freeing deferall
>   */
>  struct inet_frag_queue {
> -	spinlock_t		lock;
> +	struct rhash_head	node;
> +	union {
> +		struct frag_v4_compare_key v4;
> +		struct frag_v6_compare_key v6;
> +	} key;
>  	struct timer_list	timer;
> -	struct hlist_node	list;
> +	struct netns_frags      *net;
> +	spinlock_t		lock;

Here lock and net just change their position in struct { }.

>  	refcount_t		refcnt;
>  	struct sk_buff		*fragments;
>  	struct sk_buff		*fragments_tail;
>  	ktime_t			stamp;
>  	int			len;
>  	int			meat;
> -	__u8			flags;
> +	u8			flags;

Here just type is changed.

>  	u16			max_size;
> -	struct netns_frags	*net;
> -	struct hlist_node	list_evictor;
> -};
> -
> -#define INETFRAGS_HASHSZ	1024
> -
> -/* averaged:
> - * max_depth = default ipfrag_high_thresh / INETFRAGS_HASHSZ /
> - *	       rounded up (SKB_TRUELEN(0) + sizeof(struct ipq or
> - *	       struct frag_queue))
> - */
> -#define INETFRAGS_MAXDEPTH	128
> -
> -struct inet_frag_bucket {
> -	struct hlist_head	chain;
> -	spinlock_t		chain_lock;
> +	struct rcu_head		rcu;
>  };
>  
>  struct inet_frags {
> -	struct inet_frag_bucket	hash[INETFRAGS_HASHSZ];
> -
> -	struct work_struct	frags_work;
> -	unsigned int next_bucket;
> -	unsigned long last_rebuild_jiffies;
> -	bool rebuild;
> -
> -	/* The first call to hashfn is responsible to initialize
> -	 * rnd. This is best done with net_get_random_once.
> -	 *
> -	 * rnd_seqlock is used to let hash insertion detect
> -	 * when it needs to re-lookup the hash chain to use.
> -	 */
> -	u32			rnd;
> -	seqlock_t		rnd_seqlock;
>  	unsigned int		qsize;
>  
> -	unsigned int		(*hashfn)(const struct inet_frag_queue *);
> -	bool			(*match)(const struct inet_frag_queue *q,
> -					 const void *arg);
> -	void			(*constructor)(struct inet_frag_queue *q,
> +	void			(*constructor)(struct inet_frag_queue *fq,

Here just parameter name is changed

>  					       const void *arg);
> -	void			(*destructor)(struct inet_frag_queue *);
> +	void			(*destructor)(struct inet_frag_queue *fq);

The same as above.

>  	void			(*frag_expire)(struct timer_list *t);
>  	struct kmem_cache	*frags_cachep;
>  	const char		*frags_cache_name;
> +	struct rhashtable_params rhash_params;
>  };
>  
>  int inet_frags_init(struct inet_frags *);
> @@ -108,17 +104,13 @@ void inet_frags_fini(struct inet_frags *);
>  static inline int inet_frags_init_net(struct netns_frags *nf)
>  {
>  	atomic_set(&nf->mem, 0);
> -	return 0;
> +	return rhashtable_init(&nf->rhashtable, &nf->f->rhash_params);
>  }
>  void inet_frags_exit_net(struct netns_frags *nf);
>  
>  void inet_frag_kill(struct inet_frag_queue *q);
>  void inet_frag_destroy(struct inet_frag_queue *q);
> -struct inet_frag_queue *inet_frag_find(struct netns_frags *nf,
> -		struct inet_frags *f, void *key, unsigned int hash);
> -
> -void inet_frag_maybe_warn_overflow(struct inet_frag_queue *q,
> -				   const char *prefix);
> +struct inet_frag_queue *inet_frag_find(struct netns_frags *nf, void *key);
>  
>  static inline void inet_frag_put(struct inet_frag_queue *q)
>  {
> @@ -126,11 +118,6 @@ static inline void inet_frag_put(struct inet_frag_queue *q)
>  		inet_frag_destroy(q);
>  }
>  
> -static inline bool inet_frag_evicting(struct inet_frag_queue *q)
> -{
> -	return !hlist_unhashed(&q->list_evictor);
> -}
> -
>  /* Memory Tracking Functions. */
>  
>  static inline int frag_mem_limit(struct netns_frags *nf)
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index 5c18836672e9d1c560cdce15f5b34928c337abfd..76f84d4be91b92761fb9a26e7f52e2101ee34c0a 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -579,36 +579,20 @@ enum ip6_defrag_users {
>  	__IP6_DEFRAG_CONNTRACK_BRIDGE_IN = IP6_DEFRAG_CONNTRACK_BRIDGE_IN + USHRT_MAX,
>  };
>  
> -struct ip6_create_arg {
> -	__be32 id;
> -	u32 user;
> -	const struct in6_addr *src;
> -	const struct in6_addr *dst;
> -	int iif;
> -	u8 ecn;
> -};
> -
>  void ip6_frag_init(struct inet_frag_queue *q, const void *a);
> -bool ip6_frag_match(const struct inet_frag_queue *q, const void *a);
>  
>  /*
> - *	Equivalent of ipv4 struct ip
> + *	Equivalent of ipv4 struct ipq
>   */
>  struct frag_queue {
>  	struct inet_frag_queue	q;
>  
> -	__be32			id;		/* fragment id		*/
> -	u32			user;
> -	struct in6_addr		saddr;
> -	struct in6_addr		daddr;
> -
>  	int			iif;
>  	__u16			nhoffset;
>  	u8			ecn;
>  };
>  
> -void ip6_expire_frag_queue(struct net *net, struct frag_queue *fq,
> -			   struct inet_frags *frags);
> +void ip6_expire_frag_queue(struct net *net, struct frag_queue *fq);
>  
>  static inline bool ipv6_addr_any(const struct in6_addr *a)
>  {
> diff --git a/net/ieee802154/6lowpan/6lowpan_i.h b/net/ieee802154/6lowpan/6lowpan_i.h
> index d8de3bcfb1032a1133402cb2a4c50a2448133846..b8d95cb71c25dd69c8a88b2c886a3f0d2ce1174f 100644
> --- a/net/ieee802154/6lowpan/6lowpan_i.h
> +++ b/net/ieee802154/6lowpan/6lowpan_i.h
> @@ -17,37 +17,19 @@ typedef unsigned __bitwise lowpan_rx_result;
>  #define LOWPAN_DISPATCH_FRAG1           0xc0
>  #define LOWPAN_DISPATCH_FRAGN           0xe0
>  
> -struct lowpan_create_arg {
> +struct frag_lowpan_compare_key {
>  	u16 tag;
>  	u16 d_size;
> -	const struct ieee802154_addr *src;
> -	const struct ieee802154_addr *dst;
> +	const struct ieee802154_addr src;
> +	const struct ieee802154_addr dst;
>  };
>  
> -/* Equivalent of ipv4 struct ip
> +/* Equivalent of ipv4 struct ipq
>   */
>  struct lowpan_frag_queue {
>  	struct inet_frag_queue	q;
> -
> -	u16			tag;
> -	u16			d_size;
> -	struct ieee802154_addr	saddr;
> -	struct ieee802154_addr	daddr;
>  };
>  
> -static inline u32 ieee802154_addr_hash(const struct ieee802154_addr *a)
> -{
> -	switch (a->mode) {
> -	case IEEE802154_ADDR_LONG:
> -		return (((__force u64)a->extended_addr) >> 32) ^
> -			(((__force u64)a->extended_addr) & 0xffffffff);
> -	case IEEE802154_ADDR_SHORT:
> -		return (__force u32)(a->short_addr + (a->pan_id << 16));
> -	default:
> -		return 0;
> -	}
> -}
> -
>  int lowpan_frag_rcv(struct sk_buff *skb, const u8 frag_type);
>  void lowpan_net_frag_exit(void);
>  int lowpan_net_frag_init(void);
> diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c
> index 6badc055555b7baedac2051a1aaea15f9e9b180c..9ee4d22666c26d6d9796d0f484bb4beb265dea42 100644
> --- a/net/ieee802154/6lowpan/reassembly.c
> +++ b/net/ieee802154/6lowpan/reassembly.c
> @@ -37,47 +37,15 @@ static struct inet_frags lowpan_frags;
>  static int lowpan_frag_reasm(struct lowpan_frag_queue *fq,
>  			     struct sk_buff *prev, struct net_device *ldev);
>  
> -static unsigned int lowpan_hash_frag(u16 tag, u16 d_size,
> -				     const struct ieee802154_addr *saddr,
> -				     const struct ieee802154_addr *daddr)
> -{
> -	net_get_random_once(&lowpan_frags.rnd, sizeof(lowpan_frags.rnd));
> -	return jhash_3words(ieee802154_addr_hash(saddr),
> -			    ieee802154_addr_hash(daddr),
> -			    (__force u32)(tag + (d_size << 16)),
> -			    lowpan_frags.rnd);
> -}
> -
> -static unsigned int lowpan_hashfn(const struct inet_frag_queue *q)
> -{
> -	const struct lowpan_frag_queue *fq;
> -
> -	fq = container_of(q, struct lowpan_frag_queue, q);
> -	return lowpan_hash_frag(fq->tag, fq->d_size, &fq->saddr, &fq->daddr);
> -}
> -
> -static bool lowpan_frag_match(const struct inet_frag_queue *q, const void *a)
> -{
> -	const struct lowpan_frag_queue *fq;
> -	const struct lowpan_create_arg *arg = a;
> -
> -	fq = container_of(q, struct lowpan_frag_queue, q);
> -	return	fq->tag == arg->tag && fq->d_size == arg->d_size &&
> -		ieee802154_addr_equal(&fq->saddr, arg->src) &&
> -		ieee802154_addr_equal(&fq->daddr, arg->dst);
> -}
> -
>  static void lowpan_frag_init(struct inet_frag_queue *q, const void *a)
>  {
> -	const struct lowpan_create_arg *arg = a;
> +	const struct frag_lowpan_compare_key *key = a;
>  	struct lowpan_frag_queue *fq;
>  
>  	fq = container_of(q, struct lowpan_frag_queue, q);
>  
> -	fq->tag = arg->tag;
> -	fq->d_size = arg->d_size;
> -	fq->saddr = *arg->src;
> -	fq->daddr = *arg->dst;
> +	BUILD_BUG_ON(sizeof(*key) > sizeof(q->key));
> +	memcpy(&q->key, key, sizeof(*key));
>  }
>  
>  static void lowpan_frag_expire(struct timer_list *t)
> @@ -105,25 +73,20 @@ fq_find(struct net *net, const struct lowpan_802154_cb *cb,
>  	const struct ieee802154_addr *src,
>  	const struct ieee802154_addr *dst)
>  {
> -	struct inet_frag_queue *q;
> -	struct lowpan_create_arg arg;
> -	unsigned int hash;
>  	struct netns_ieee802154_lowpan *ieee802154_lowpan =
>  		net_ieee802154_lowpan(net);
> +	struct frag_lowpan_compare_key key = {
> +		.tag = cb->d_tag,
> +		.d_size = cb->d_size,
> +		.src = *src,
> +		.dst = *dst,
> +	};
> +	struct inet_frag_queue *q;
>  
> -	arg.tag = cb->d_tag;
> -	arg.d_size = cb->d_size;
> -	arg.src = src;
> -	arg.dst = dst;
> -
> -	hash = lowpan_hash_frag(cb->d_tag, cb->d_size, src, dst);
> -
> -	q = inet_frag_find(&ieee802154_lowpan->frags,
> -			   &lowpan_frags, &arg, hash);
> -	if (IS_ERR_OR_NULL(q)) {
> -		inet_frag_maybe_warn_overflow(q, pr_fmt());
> +	q = inet_frag_find(&ieee802154_lowpan->frags, &key);
> +	if (IS_ERR_OR_NULL(q))
>  		return NULL;
> -	}
> +
>  	return container_of(q, struct lowpan_frag_queue, q);
>  }
>  
> @@ -588,6 +551,7 @@ static int __net_init lowpan_frags_init_net(struct net *net)
>  	ieee802154_lowpan->frags.timeout = IPV6_FRAG_TIMEOUT;
>  	ieee802154_lowpan->frags.f = &lowpan_frags;
>  
> +	ieee802154_lowpan->frags.f = &lowpan_frags;
>  	res = inet_frags_init_net(&ieee802154_lowpan->frags);
>  	if (res < 0)
>  		return res;
> @@ -611,6 +575,36 @@ static struct pernet_operations lowpan_frags_ops = {
>  	.exit = lowpan_frags_exit_net,
>  };
>  
> +static u32 lowpan_key_hashfn(const void *data, u32 len, u32 seed)
> +{
> +	return jhash2(data,
> +		      sizeof(struct frag_lowpan_compare_key) / sizeof(u32), seed);
> +}
> +
> +static u32 lowpan_obj_hashfn(const void *data, u32 len, u32 seed)
> +{
> +	const struct inet_frag_queue *fq = data;
> +
> +	return jhash2((const u32 *)&fq->key,
> +		      sizeof(struct frag_lowpan_compare_key) / sizeof(u32), seed);
> +}
> +
> +static int lowpan_obj_cmpfn(struct rhashtable_compare_arg *arg, const void *ptr)
> +{
> +	const struct frag_lowpan_compare_key *key = arg->key;
> +	const struct inet_frag_queue *fq = ptr;
> +
> +	return !!memcmp(&fq->key, key, sizeof(*key));
> +}
> +
> +const struct rhashtable_params lowpan_rhash_params = {
> +	.head_offset		= offsetof(struct inet_frag_queue, node),
> +	.hashfn			= lowpan_key_hashfn,
> +	.obj_hashfn		= lowpan_obj_hashfn,
> +	.obj_cmpfn		= lowpan_obj_cmpfn,
> +	.automatic_shrinking	= true,
> +};
> +
>  int __init lowpan_net_frag_init(void)
>  {
>  	int ret;
> @@ -619,22 +613,24 @@ int __init lowpan_net_frag_init(void)
>  	if (ret)
>  		return ret;
>  
> -	ret = register_pernet_subsys(&lowpan_frags_ops);
> -	if (ret)
> -		goto err_pernet;
> -
> -	lowpan_frags.hashfn = lowpan_hashfn;
>  	lowpan_frags.constructor = lowpan_frag_init;
>  	lowpan_frags.destructor = NULL;
>  	lowpan_frags.qsize = sizeof(struct frag_queue);
> -	lowpan_frags.match = lowpan_frag_match;
>  	lowpan_frags.frag_expire = lowpan_frag_expire;
>  	lowpan_frags.frags_cache_name = lowpan_frags_cache_name;
> +	lowpan_frags.rhash_params = lowpan_rhash_params;
>  	ret = inet_frags_init(&lowpan_frags);
>  	if (ret)
>  		goto err_pernet;
>  
> +	ret = register_pernet_subsys(&lowpan_frags_ops);
> +	if (ret)
> +		goto err_pernet_frags;
> +

Can't we move this register_pernet_subsys() in separate patch?

>  	return ret;
> +
> +err_pernet_frags:
> +	inet_frags_fini(&lowpan_frags);
>  err_pernet:
>  	lowpan_frags_sysctl_unregister();
>  	return ret;
> diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> index 1ac69f65d0dee600d0ab4db20ff5942952932c40..8ccaf605630f14270996ee1b5a37376299d78661 100644
> --- a/net/ipv4/inet_fragment.c
> +++ b/net/ipv4/inet_fragment.c
> @@ -25,12 +25,6 @@
>  #include <net/inet_frag.h>
>  #include <net/inet_ecn.h>
>  
> -#define INETFRAGS_EVICT_BUCKETS   128
> -#define INETFRAGS_EVICT_MAX	  512
> -
> -/* don't rebuild inetfrag table with new secret more often than this */
> -#define INETFRAGS_MIN_REBUILD_INTERVAL (5 * HZ)
> -
>  /* Given the OR values of all fragments, apply RFC 3168 5.3 requirements
>   * Value : 0xff if frame should be dropped.
>   *         0 or INET_ECN_CE value, to be ORed in to final iph->tos field
> @@ -52,157 +46,8 @@ const u8 ip_frag_ecn_table[16] = {
>  };
>  EXPORT_SYMBOL(ip_frag_ecn_table);
>  
> -static unsigned int
> -inet_frag_hashfn(const struct inet_frags *f, const struct inet_frag_queue *q)
> -{
> -	return f->hashfn(q) & (INETFRAGS_HASHSZ - 1);
> -}
> -
> -static bool inet_frag_may_rebuild(struct inet_frags *f)
> -{
> -	return time_after(jiffies,
> -	       f->last_rebuild_jiffies + INETFRAGS_MIN_REBUILD_INTERVAL);
> -}
> -
> -static void inet_frag_secret_rebuild(struct inet_frags *f)
> -{
> -	int i;
> -
> -	write_seqlock_bh(&f->rnd_seqlock);
> -
> -	if (!inet_frag_may_rebuild(f))
> -		goto out;
> -
> -	get_random_bytes(&f->rnd, sizeof(u32));
> -
> -	for (i = 0; i < INETFRAGS_HASHSZ; i++) {
> -		struct inet_frag_bucket *hb;
> -		struct inet_frag_queue *q;
> -		struct hlist_node *n;
> -
> -		hb = &f->hash[i];
> -		spin_lock(&hb->chain_lock);
> -
> -		hlist_for_each_entry_safe(q, n, &hb->chain, list) {
> -			unsigned int hval = inet_frag_hashfn(f, q);
> -
> -			if (hval != i) {
> -				struct inet_frag_bucket *hb_dest;
> -
> -				hlist_del(&q->list);
> -
> -				/* Relink to new hash chain. */
> -				hb_dest = &f->hash[hval];
> -
> -				/* This is the only place where we take
> -				 * another chain_lock while already holding
> -				 * one.  As this will not run concurrently,
> -				 * we cannot deadlock on hb_dest lock below, if its
> -				 * already locked it will be released soon since
> -				 * other caller cannot be waiting for hb lock
> -				 * that we've taken above.
> -				 */
> -				spin_lock_nested(&hb_dest->chain_lock,
> -						 SINGLE_DEPTH_NESTING);
> -				hlist_add_head(&q->list, &hb_dest->chain);
> -				spin_unlock(&hb_dest->chain_lock);
> -			}
> -		}
> -		spin_unlock(&hb->chain_lock);
> -	}
> -
> -	f->rebuild = false;
> -	f->last_rebuild_jiffies = jiffies;
> -out:
> -	write_sequnlock_bh(&f->rnd_seqlock);
> -}
> -
> -static bool inet_fragq_should_evict(const struct inet_frag_queue *q)
> -{
> -	if (!hlist_unhashed(&q->list_evictor))
> -		return false;
> -
> -	return q->net->low_thresh == 0 ||
> -	       frag_mem_limit(q->net) >= q->net->low_thresh;
> -}
> -
> -static unsigned int
> -inet_evict_bucket(struct inet_frags *f, struct inet_frag_bucket *hb)
> -{
> -	struct inet_frag_queue *fq;
> -	struct hlist_node *n;
> -	unsigned int evicted = 0;
> -	HLIST_HEAD(expired);
> -
> -	spin_lock(&hb->chain_lock);
> -
> -	hlist_for_each_entry_safe(fq, n, &hb->chain, list) {
> -		if (!inet_fragq_should_evict(fq))
> -			continue;
> -
> -		if (!del_timer(&fq->timer))
> -			continue;
> -
> -		hlist_add_head(&fq->list_evictor, &expired);
> -		++evicted;
> -	}
> -
> -	spin_unlock(&hb->chain_lock);
> -
> -	hlist_for_each_entry_safe(fq, n, &expired, list_evictor)
> -		f->frag_expire(&fq->timer);
> -
> -	return evicted;
> -}
> -
> -static void inet_frag_worker(struct work_struct *work)
> -{
> -	unsigned int budget = INETFRAGS_EVICT_BUCKETS;
> -	unsigned int i, evicted = 0;
> -	struct inet_frags *f;
> -
> -	f = container_of(work, struct inet_frags, frags_work);
> -
> -	BUILD_BUG_ON(INETFRAGS_EVICT_BUCKETS >= INETFRAGS_HASHSZ);
> -
> -	local_bh_disable();
> -
> -	for (i = READ_ONCE(f->next_bucket); budget; --budget) {
> -		evicted += inet_evict_bucket(f, &f->hash[i]);
> -		i = (i + 1) & (INETFRAGS_HASHSZ - 1);
> -		if (evicted > INETFRAGS_EVICT_MAX)
> -			break;
> -	}
> -
> -	f->next_bucket = i;
> -
> -	local_bh_enable();
> -
> -	if (f->rebuild && inet_frag_may_rebuild(f))
> -		inet_frag_secret_rebuild(f);
> -}
> -
> -static void inet_frag_schedule_worker(struct inet_frags *f)
> -{
> -	if (unlikely(!work_pending(&f->frags_work)))
> -		schedule_work(&f->frags_work);
> -}
> -
>  int inet_frags_init(struct inet_frags *f)
>  {
> -	int i;
> -
> -	INIT_WORK(&f->frags_work, inet_frag_worker);
> -
> -	for (i = 0; i < INETFRAGS_HASHSZ; i++) {
> -		struct inet_frag_bucket *hb = &f->hash[i];
> -
> -		spin_lock_init(&hb->chain_lock);
> -		INIT_HLIST_HEAD(&hb->chain);
> -	}
> -
> -	seqlock_init(&f->rnd_seqlock);
> -	f->last_rebuild_jiffies = 0;
>  	f->frags_cachep = kmem_cache_create(f->frags_cache_name, f->qsize, 0, 0,
>  					    NULL);
>  	if (!f->frags_cachep)
> @@ -214,93 +59,80 @@ EXPORT_SYMBOL(inet_frags_init);
>  
>  void inet_frags_fini(struct inet_frags *f)
>  {
> -	cancel_work_sync(&f->frags_work);
> +	rcu_barrier();

What does this barrier waits? This should have a comment.

>  	kmem_cache_destroy(f->frags_cachep);
> +	f->frags_cachep = NULL;
>  }
>  EXPORT_SYMBOL(inet_frags_fini);
>  
>  void inet_frags_exit_net(struct netns_frags *nf)
>  {
> -	struct inet_frags *f =nf->f;
> -	unsigned int seq;
> -	int i;
> -
> -	nf->low_thresh = 0;
> +	struct rhashtable_iter hti;
> +	struct inet_frag_queue *fq;
>  
> -evict_again:
> -	local_bh_disable();
> -	seq = read_seqbegin(&f->rnd_seqlock);
> +	nf->low_thresh = 0; /* prevent creation of new frags */
>  
> -	for (i = 0; i < INETFRAGS_HASHSZ ; i++)
> -		inet_evict_bucket(f, &f->hash[i]);
> +	rhashtable_walk_enter(&nf->rhashtable, &hti);
> +	do {
> +		rhashtable_walk_start(&hti);
>  
> -	local_bh_enable();
> -	cond_resched();
> +		while ((fq = rhashtable_walk_next(&hti)) && !IS_ERR(fq)) {
> +			if (refcount_inc_not_zero(&fq->refcnt)) {
> +				spin_lock_bh(&fq->lock);
> +				inet_frag_kill(fq);
> +				spin_unlock_bh(&fq->lock);
> +				inet_frag_put(fq);
> +			}
> +		}
>  
> -	if (read_seqretry(&f->rnd_seqlock, seq) ||
> -	    sum_frag_mem_limit(nf))
> -		goto evict_again;
> +		rhashtable_walk_stop(&hti);
> +	} while (cond_resched(), fq == ERR_PTR(-EAGAIN));
> +	rhashtable_walk_exit(&hti);
> +	rhashtable_destroy(&nf->rhashtable);
>  }
>  EXPORT_SYMBOL(inet_frags_exit_net);
>  
> -static struct inet_frag_bucket *
> -get_frag_bucket_locked(struct inet_frag_queue *fq, struct inet_frags *f)
> -__acquires(hb->chain_lock)
> -{
> -	struct inet_frag_bucket *hb;
> -	unsigned int seq, hash;
> -
> - restart:
> -	seq = read_seqbegin(&f->rnd_seqlock);
> -
> -	hash = inet_frag_hashfn(f, fq);
> -	hb = &f->hash[hash];
> -
> -	spin_lock(&hb->chain_lock);
> -	if (read_seqretry(&f->rnd_seqlock, seq)) {
> -		spin_unlock(&hb->chain_lock);
> -		goto restart;
> -	}
> -
> -	return hb;
> -}
> -
> -static inline void fq_unlink(struct inet_frag_queue *fq)
> -{
> -	struct inet_frag_bucket *hb;
> -
> -	hb = get_frag_bucket_locked(fq, fq->net->f);
> -	hlist_del(&fq->list);
> -	fq->flags |= INET_FRAG_COMPLETE;
> -	spin_unlock(&hb->chain_lock);
> -}
> -
>  void inet_frag_kill(struct inet_frag_queue *fq)
>  {
>  	if (del_timer(&fq->timer))
>  		refcount_dec(&fq->refcnt);
>  
>  	if (!(fq->flags & INET_FRAG_COMPLETE)) {
> -		fq_unlink(fq);
> +		struct netns_frags *nf = fq->net;
> +
> +		fq->flags |= INET_FRAG_COMPLETE;
> +		rhashtable_remove_fast(&nf->rhashtable, &fq->node, nf->f->rhash_params);
>  		refcount_dec(&fq->refcnt);
>  	}
>  }
>  EXPORT_SYMBOL(inet_frag_kill);
>  
> -void inet_frag_destroy(struct inet_frag_queue *q)
> +static void inet_frag_destroy_rcu(struct rcu_head *head)
>  {
> -	struct sk_buff *fp;
> +	struct inet_frag_queue *fq = container_of(head, struct inet_frag_queue,
> +						 rcu);
> +	struct inet_frags *f = fq->net->f;
> +
> +	if (f->destructor)
> +		f->destructor(fq);
> +	kmem_cache_free(f->frags_cachep, fq);
> +}
> +
> +void inet_frag_destroy(struct inet_frag_queue *fq)
> +{
> +	unsigned int sum_truesize;
>  	struct netns_frags *nf;
> -	unsigned int sum, sum_truesize = 0;
>  	struct inet_frags *f;
> +	struct sk_buff *fp;
>  
> -	WARN_ON(!(q->flags & INET_FRAG_COMPLETE));
> -	WARN_ON(del_timer(&q->timer) != 0);
> +	WARN_ON(!(fq->flags & INET_FRAG_COMPLETE));
> +	WARN_ON(del_timer(&fq->timer) != 0);

This is actually a result of renaming the variable.
The type of variable remains the same, while the name
has changed. So, this should go in separate patch.

>  
>  	/* Release all fragment data. */
> -	fp = q->fragments;
> -	nf = q->net;
> +	fp = fq->fragments;
> +	nf = fq->net;

The same here

>  	f = nf->f;
> +	sum_truesize = f->qsize;
>  	while (fp) {
>  		struct sk_buff *xp = fp->next;
>  
> @@ -308,136 +140,63 @@ void inet_frag_destroy(struct inet_frag_queue *q)
>  		kfree_skb(fp);
>  		fp = xp;
>  	}
> -	sum = sum_truesize + f->qsize;
>  
> -	if (f->destructor)
> -		f->destructor(q);
> -	kmem_cache_free(f->frags_cachep, q);
> +	call_rcu(&fq->rcu, inet_frag_destroy_rcu);
>  
> -	sub_frag_mem_limit(nf, sum);
> +	sub_frag_mem_limit(nf, sum_truesize);
>  }
>  EXPORT_SYMBOL(inet_frag_destroy);
>  
> -static struct inet_frag_queue *inet_frag_intern(struct netns_frags *nf,
> -						struct inet_frag_queue *qp_in,
> -						struct inet_frags *f,
> +static struct inet_frag_queue *inet_frag_create(struct netns_frags *nf,
>  						void *arg)
>  {
> -	struct inet_frag_bucket *hb = get_frag_bucket_locked(qp_in, f);
> -	struct inet_frag_queue *qp;
> -
> -#ifdef CONFIG_SMP
> -	/* With SMP race we have to recheck hash table, because
> -	 * such entry could have been created on other cpu before
> -	 * we acquired hash bucket lock.
> -	 */
> -	hlist_for_each_entry(qp, &hb->chain, list) {
> -		if (qp->net == nf && f->match(qp, arg)) {
> -			refcount_inc(&qp->refcnt);
> -			spin_unlock(&hb->chain_lock);
> -			qp_in->flags |= INET_FRAG_COMPLETE;
> -			inet_frag_put(qp_in);
> -			return qp;
> -		}
> -	}
> -#endif
> -	qp = qp_in;
> -	if (!mod_timer(&qp->timer, jiffies + nf->timeout))
> -		refcount_inc(&qp->refcnt);
> -
> -	refcount_inc(&qp->refcnt);
> -	hlist_add_head(&qp->list, &hb->chain);
> -
> -	spin_unlock(&hb->chain_lock);
> -
> -	return qp;
> -}
> -
> -static struct inet_frag_queue *inet_frag_alloc(struct netns_frags *nf,
> -					       struct inet_frags *f,
> -					       void *arg)
> -{
> -	struct inet_frag_queue *q;
> +	struct inet_frags *f = nf->f;
> +	struct inet_frag_queue *fq;
> +	int err;
>  
> -	if (!nf->high_thresh || frag_mem_limit(nf) > nf->high_thresh) {
> -		inet_frag_schedule_worker(f);
> +	if (!nf->high_thresh || frag_mem_limit(nf) > nf->high_thresh)
>  		return NULL;
> -	}
>  
> -	q = kmem_cache_zalloc(f->frags_cachep, GFP_ATOMIC);
> -	if (!q)
> +	fq = kmem_cache_zalloc(f->frags_cachep, GFP_ATOMIC);
> +	if (!fq)
>  		return NULL;

Here we also renamed a variable and merged two functions: inet_frag_alloc()
into inet_frag_create(). Can't we do that in separate patch?

Also, note, git (at least my) generates better diff for this hunk
with the following options in .gitconfig:

[diff "default"]
        algorithm = patience
[diff]
        algorithm = patience

>  
> -	q->net = nf;
> -	f->constructor(q, arg);
> -	add_frag_mem_limit(nf, f->qsize);
> -
> -	timer_setup(&q->timer, f->frag_expire, 0);
> -	spin_lock_init(&q->lock);
> -	refcount_set(&q->refcnt, 1);
> -
> -	return q;
> -}
> +	fq->net = nf;
> +	f->constructor(fq, arg);

Also renaming.

>  
> -static struct inet_frag_queue *inet_frag_create(struct netns_frags *nf,
> -						struct inet_frags *f,
> -						void *arg)
> -{
> -	struct inet_frag_queue *q;
> +	timer_setup(&fq->timer, f->frag_expire, 0);
> +	spin_lock_init(&fq->lock);
> +	refcount_set(&fq->refcnt, 3);
> +	mod_timer(&fq->timer, jiffies + nf->timeout);
>  
> -	q = inet_frag_alloc(nf, f, arg);
> -	if (!q)
> +	err = rhashtable_insert_fast(&nf->rhashtable, &fq->node,
> +				     f->rhash_params);
> +	add_frag_mem_limit(nf, f->qsize);
> +	if (err < 0) {
> +		fq->flags |= INET_FRAG_COMPLETE;
> +		inet_frag_kill(fq);
> +		inet_frag_destroy(fq);
>  		return NULL;
> -
> -	return inet_frag_intern(nf, q, f, arg);
> +	}
> +	return fq;
>  }
>  
> -struct inet_frag_queue *inet_frag_find(struct netns_frags *nf,
> -				       struct inet_frags *f, void *key,
> -				       unsigned int hash)
> +/* TODO : call from rcu_read_lock() and no longer use refcount_inc_not_zero() */
> +struct inet_frag_queue *inet_frag_find(struct netns_frags *nf, void *key)
>  {
> -	struct inet_frag_bucket *hb;
> -	struct inet_frag_queue *q;
> -	int depth = 0;
> -
> -	if (frag_mem_limit(nf) > nf->low_thresh)
> -		inet_frag_schedule_worker(f);
> -
> -	hash &= (INETFRAGS_HASHSZ - 1);
> -	hb = &f->hash[hash];
> -
> -	spin_lock(&hb->chain_lock);
> -	hlist_for_each_entry(q, &hb->chain, list) {
> -		if (q->net == nf && f->match(q, key)) {
> -			refcount_inc(&q->refcnt);
> -			spin_unlock(&hb->chain_lock);
> -			return q;
> -		}
> -		depth++;
> -	}
> -	spin_unlock(&hb->chain_lock);
> +	struct inet_frag_queue *fq;
>  
> -	if (depth <= INETFRAGS_MAXDEPTH)
> -		return inet_frag_create(nf, f, key);
> +	rcu_read_lock();
>  
> -	if (inet_frag_may_rebuild(f)) {
> -		if (!f->rebuild)
> -			f->rebuild = true;
> -		inet_frag_schedule_worker(f);
> +	fq = rhashtable_lookup(&nf->rhashtable, key, nf->f->rhash_params);
> +	if (fq) {
> +		if (!refcount_inc_not_zero(&fq->refcnt))
> +			fq = NULL;
> +		rcu_read_unlock();
> +		return fq;
>  	}
> +	rcu_read_unlock();
>  
> -	return ERR_PTR(-ENOBUFS);
> +	return inet_frag_create(nf, key);
>  }
>  EXPORT_SYMBOL(inet_frag_find);
> -
> -void inet_frag_maybe_warn_overflow(struct inet_frag_queue *q,
> -				   const char *prefix)
> -{
> -	static const char msg[] = "inet_frag_find: Fragment hash bucket"
> -		" list length grew over limit " __stringify(INETFRAGS_MAXDEPTH)
> -		". Dropping fragment.\n";
> -
> -	if (PTR_ERR(q) == -ENOBUFS)
> -		net_dbg_ratelimited("%s%s", prefix, msg);
> -}
> -EXPORT_SYMBOL(inet_frag_maybe_warn_overflow);
> diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
> index cd2b4c9419fc1552d367b572926e314b11cb6c00..1a7423e8ec0a8f88782ad8c945dc0cd6046f79f0 100644
> --- a/net/ipv4/ip_fragment.c
> +++ b/net/ipv4/ip_fragment.c
> @@ -69,15 +69,9 @@ struct ipfrag_skb_cb
>  struct ipq {
>  	struct inet_frag_queue q;
>  
> -	u32		user;
> -	__be32		saddr;
> -	__be32		daddr;
> -	__be16		id;
> -	u8		protocol;
>  	u8		ecn; /* RFC3168 support */
>  	u16		max_df_size; /* largest frag with DF set seen */
>  	int             iif;
> -	int             vif;   /* L3 master device index */
>  	unsigned int    rid;
>  	struct inet_peer *peer;
>  };
> @@ -97,41 +91,6 @@ int ip_frag_mem(struct net *net)
>  static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev,
>  			 struct net_device *dev);
>  
> -struct ip4_create_arg {
> -	struct iphdr *iph;
> -	u32 user;
> -	int vif;
> -};
> -
> -static unsigned int ipqhashfn(__be16 id, __be32 saddr, __be32 daddr, u8 prot)
> -{
> -	net_get_random_once(&ip4_frags.rnd, sizeof(ip4_frags.rnd));
> -	return jhash_3words((__force u32)id << 16 | prot,
> -			    (__force u32)saddr, (__force u32)daddr,
> -			    ip4_frags.rnd);
> -}
> -
> -static unsigned int ip4_hashfn(const struct inet_frag_queue *q)
> -{
> -	const struct ipq *ipq;
> -
> -	ipq = container_of(q, struct ipq, q);
> -	return ipqhashfn(ipq->id, ipq->saddr, ipq->daddr, ipq->protocol);
> -}
> -
> -static bool ip4_frag_match(const struct inet_frag_queue *q, const void *a)
> -{
> -	const struct ipq *qp;
> -	const struct ip4_create_arg *arg = a;
> -
> -	qp = container_of(q, struct ipq, q);
> -	return	qp->id == arg->iph->id &&
> -		qp->saddr == arg->iph->saddr &&
> -		qp->daddr == arg->iph->daddr &&
> -		qp->protocol == arg->iph->protocol &&
> -		qp->user == arg->user &&
> -		qp->vif == arg->vif;
> -}
>  
>  static void ip4_frag_init(struct inet_frag_queue *q, const void *a)
>  {
> @@ -140,37 +99,23 @@ static void ip4_frag_init(struct inet_frag_queue *q, const void *a)
>  					       frags);
>  	struct net *net = container_of(ipv4, struct net, ipv4);
>  
> -	const struct ip4_create_arg *arg = a;
> +	const struct frag_v4_compare_key *key = a;
>  
> -	qp->protocol = arg->iph->protocol;
> -	qp->id = arg->iph->id;
> -	qp->ecn = ip4_frag_ecn(arg->iph->tos);
> -	qp->saddr = arg->iph->saddr;
> -	qp->daddr = arg->iph->daddr;
> -	qp->vif = arg->vif;
> -	qp->user = arg->user;
> +	q->key.v4 = *key;
> +	qp->ecn = 0;
>  	qp->peer = q->net->max_dist ?
> -		inet_getpeer_v4(net->ipv4.peers, arg->iph->saddr, arg->vif, 1) :
> +		inet_getpeer_v4(net->ipv4.peers, key->saddr, key->vif, 1) :
>  		NULL;
>  }
>  
> -static void ip4_frag_free(struct inet_frag_queue *q)
> +static void ip4_frag_destructor(struct inet_frag_queue *q)

This just renames the function

>  {
> -	struct ipq *qp;
> +	struct ipq *qp = container_of(q, struct ipq, q);
>  
> -	qp = container_of(q, struct ipq, q);

This is also just a refactoring

>  	if (qp->peer)
>  		inet_putpeer(qp->peer);
>  }
>  
> -
> -/* Destruction primitives. */
> -
> -static void ipq_put(struct ipq *ipq)
> -{
> -	inet_frag_put(&ipq->q);
> -}
> -
>  /* Kill ipq entry. It is not destroyed immediately,
>   * because caller (and someone more) holds reference count.
>   */
> @@ -198,25 +143,25 @@ static void ip_expire(struct timer_list *t)
>  	struct net *net;
>  
>  	qp = container_of(frag, struct ipq, q);
> -	net = container_of(qp->q.net, struct net, ipv4.frags);
> +	net = container_of(frag->net, struct net, ipv4.frags);
>  
>  	rcu_read_lock();
> -	spin_lock(&qp->q.lock);
> +	spin_lock(&frag->lock);
>  
> -	if (qp->q.flags & INET_FRAG_COMPLETE)
> +	if (frag->flags & INET_FRAG_COMPLETE)
>  		goto out;
>  
>  	ipq_kill(qp);
>  	__IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS);
>  
> -	if (!inet_frag_evicting(&qp->q)) {
> -		struct sk_buff *clone, *head = qp->q.fragments;
> +	if (true) {

This does not look good for me. Better we could try to move this
to separate function..

> +		struct sk_buff *clone, *head = frag->fragments;
>  		const struct iphdr *iph;
>  		int err;
>  
>  		__IP_INC_STATS(net, IPSTATS_MIB_REASMTIMEOUT);
>  
> -		if (!(qp->q.flags & INET_FRAG_FIRST_IN) || !qp->q.fragments)
> +		if (!(frag->flags & INET_FRAG_FIRST_IN) || !frag->fragments)
>  			goto out;
>  
>  		head->dev = dev_get_by_index_rcu(net, qp->iif);
> @@ -234,7 +179,7 @@ static void ip_expire(struct timer_list *t)
>  		/* Only an end host needs to send an ICMP
>  		 * "Fragment Reassembly Timeout" message, per RFC792.
>  		 */
> -		if (frag_expire_skip_icmp(qp->user) &&
> +		if (frag_expire_skip_icmp(frag->key.v4.user) &&
>  		    (skb_rtable(head)->rt_type != RTN_LOCAL))
>  			goto out;
>  
> @@ -242,7 +187,7 @@ static void ip_expire(struct timer_list *t)
>  
>  		/* Send an ICMP "Fragment Reassembly Timeout" message. */
>  		if (clone) {
> -			spin_unlock(&qp->q.lock);
> +			spin_unlock(&frag->lock);
>  			icmp_send(clone, ICMP_TIME_EXCEEDED,
>  				  ICMP_EXC_FRAGTIME, 0);
>  			consume_skb(clone);
> @@ -250,33 +195,32 @@ static void ip_expire(struct timer_list *t)
>  		}
>  	}
>  out:
> -	spin_unlock(&qp->q.lock);
> +	spin_unlock(&frag->lock);
>  out_rcu_unlock:
>  	rcu_read_unlock();
> -	ipq_put(qp);
> +	inet_frag_put(frag);
>  }
>  
>  /* Find the correct entry in the "incomplete datagrams" queue for
>   * this IP datagram, and create new one, if nothing is found.
>   */
> -static struct ipq *ip_find(struct net *net, struct iphdr *iph,
> +static struct ipq *ip_find(struct net *net, const struct iphdr *iph,
>  			   u32 user, int vif)
>  {
> +	struct frag_v4_compare_key key = {
> +		.saddr = iph->saddr,
> +		.daddr = iph->daddr,
> +		.user = user,
> +		.vif = vif,
> +		.id = iph->id,
> +		.protocol = iph->protocol,
> +	};
>  	struct inet_frag_queue *q;
> -	struct ip4_create_arg arg;
> -	unsigned int hash;
>  
> -	arg.iph = iph;
> -	arg.user = user;
> -	arg.vif = vif;
> -
> -	hash = ipqhashfn(iph->id, iph->saddr, iph->daddr, iph->protocol);
> -
> -	q = inet_frag_find(&net->ipv4.frags, &ip4_frags, &arg, hash);
> -	if (IS_ERR_OR_NULL(q)) {
> -		inet_frag_maybe_warn_overflow(q, pr_fmt());
> +	q = inet_frag_find(&net->ipv4.frags, &key);
> +	if (!q)
>  		return NULL;
> -	}
> +
>  	return container_of(q, struct ipq, q);
>  }
>  
> @@ -310,8 +254,8 @@ static int ip_frag_too_far(struct ipq *qp)
>  
>  static int ip_frag_reinit(struct ipq *qp)
>  {
> -	struct sk_buff *fp;
>  	unsigned int sum_truesize = 0;
> +	struct sk_buff *fp;

This just moves the line...

>  
>  	if (!mod_timer(&qp->q.timer, jiffies + qp->q.net->timeout)) {
>  		refcount_inc(&qp->q.refcnt);
> @@ -652,11 +596,11 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev,
>  	return 0;
>  
>  out_nomem:
> -	net_dbg_ratelimited("queue_glue: no memory for gluing queue %p\n", qp);
> +	net_dbg_ratelimited("queue_glue: no memory for gluing queue\n");

Since there is no parameter type change, but complete removing of it,
we may do that in refatoring patch (together with the above).

>  	err = -ENOMEM;
>  	goto out_fail;
>  out_oversize:
> -	net_info_ratelimited("Oversized IP packet from %pI4\n", &qp->saddr);
> +	net_info_ratelimited("Oversized IP packet from %pI4\n", &qp->q.key.v4.saddr);
>  out_fail:
>  	__IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS);
>  	return err;
> @@ -682,7 +626,7 @@ int ip_defrag(struct net *net, struct sk_buff *skb, u32 user)
>  		ret = ip_frag_queue(qp, skb);
>  
>  		spin_unlock(&qp->q.lock);
> -		ipq_put(qp);
> +		inet_frag_put(&qp->q);
>  		return ret;
>  	}
>  
> @@ -894,17 +838,52 @@ static struct pernet_operations ip4_frags_ops = {
>  	.exit = ipv4_frags_exit_net,
>  };
>  
> +
> +static u32 ip4_key_hashfn(const void *data, u32 len, u32 seed)
> +{
> +	return jhash2(data,
> +		      sizeof(struct frag_v4_compare_key) / sizeof(u32), seed);
> +}
> +
> +static u32 ip4_obj_hashfn(const void *data, u32 len, u32 seed)
> +{
> +	const struct inet_frag_queue *fq = data;
> +
> +	return jhash2((const u32 *)&fq->key.v4,
> +		      sizeof(struct frag_v4_compare_key) / sizeof(u32), seed);
> +}
> +
> +static int ip4_obj_cmpfn(struct rhashtable_compare_arg *arg, const void *ptr)
> +{
> +	const struct frag_v4_compare_key *key = arg->key;
> +	const struct inet_frag_queue *fq = ptr;
> +
> +	return !!memcmp(&fq->key, key, sizeof(*key));
> +}
> +
> +static const struct rhashtable_params ip4_rhash_params = {
> +	.head_offset		= offsetof(struct inet_frag_queue, node),
> +	.key_offset		= offsetof(struct inet_frag_queue, key),
> +	.key_len		= sizeof(struct frag_v4_compare_key),
> +	.hashfn			= ip4_key_hashfn,
> +	.obj_hashfn		= ip4_obj_hashfn,
> +	.obj_cmpfn		= ip4_obj_cmpfn,
> +	.automatic_shrinking	= true,
> +};
> +
>  void __init ipfrag_init(void)
>  {
> -	ip4_frags_ctl_register();
> -	register_pernet_subsys(&ip4_frags_ops);
> -	ip4_frags.hashfn = ip4_hashfn;
>  	ip4_frags.constructor = ip4_frag_init;
> -	ip4_frags.destructor = ip4_frag_free;
> +	ip4_frags.destructor = ip4_frag_destructor;

Here just reflects the fact we removed the function

>  	ip4_frags.qsize = sizeof(struct ipq);
> -	ip4_frags.match = ip4_frag_match;
>  	ip4_frags.frag_expire = ip_expire;
>  	ip4_frags.frags_cache_name = ip_frag_cache_name;
> +	ip4_frags.rhash_params = ip4_rhash_params;
> +
>  	if (inet_frags_init(&ip4_frags))
>  		panic("IP: failed to allocate ip4_frags cache\n");
> +
> +	ip4_frags_ctl_register();
> +	register_pernet_subsys(&ip4_frags_ops);

We may consider to do this moving in refatoring patch..

> +
>  }
> diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
> index f69b7ca52727c814eb2887c9deb9f356c56e5442..53859311dea96c03fa5ae8456de32de25009efbe 100644
> --- a/net/ipv6/netfilter/nf_conntrack_reasm.c
> +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
> @@ -152,23 +152,6 @@ static inline u8 ip6_frag_ecn(const struct ipv6hdr *ipv6h)
>  	return 1 << (ipv6_get_dsfield(ipv6h) & INET_ECN_MASK);
>  }
>  
> -static unsigned int nf_hash_frag(__be32 id, const struct in6_addr *saddr,
> -				 const struct in6_addr *daddr)
> -{
> -	net_get_random_once(&nf_frags.rnd, sizeof(nf_frags.rnd));
> -	return jhash_3words(ipv6_addr_hash(saddr), ipv6_addr_hash(daddr),
> -			    (__force u32)id, nf_frags.rnd);
> -}
> -
> -
> -static unsigned int nf_hashfn(const struct inet_frag_queue *q)
> -{
> -	const struct frag_queue *nq;
> -
> -	nq = container_of(q, struct frag_queue, q);
> -	return nf_hash_frag(nq->id, &nq->saddr, &nq->daddr);
> -}
> -
>  static void nf_ct_frag6_expire(struct timer_list *t)
>  {
>  	struct inet_frag_queue *frag = from_timer(frag, t, timer);
> @@ -178,34 +161,26 @@ static void nf_ct_frag6_expire(struct timer_list *t)
>  	fq = container_of(frag, struct frag_queue, q);
>  	net = container_of(fq->q.net, struct net, nf_frag.frags);
>  
> -	ip6_expire_frag_queue(net, fq, &nf_frags);
> +	ip6_expire_frag_queue(net, fq);
>  }
>  
>  /* Creation primitives. */
> -static inline struct frag_queue *fq_find(struct net *net, __be32 id,
> -					 u32 user, struct in6_addr *src,
> -					 struct in6_addr *dst, int iif, u8 ecn)
> +static struct frag_queue *fq_find(struct net *net, __be32 id, u32 user,
> +				  const struct ipv6hdr *hdr, int iif)
>  {
> +	struct frag_v6_compare_key key = {
> +		.id = id,
> +		.saddr = hdr->saddr,
> +		.daddr = hdr->daddr,
> +		.user = user,
> +		.iif = iif,
> +	};
>  	struct inet_frag_queue *q;
> -	struct ip6_create_arg arg;
> -	unsigned int hash;
> -
> -	arg.id = id;
> -	arg.user = user;
> -	arg.src = src;
> -	arg.dst = dst;
> -	arg.iif = iif;
> -	arg.ecn = ecn;
> -
> -	local_bh_disable();
> -	hash = nf_hash_frag(id, src, dst);
> -
> -	q = inet_frag_find(&net->nf_frag.frags, &nf_frags, &arg, hash);
> -	local_bh_enable();
> -	if (IS_ERR_OR_NULL(q)) {
> -		inet_frag_maybe_warn_overflow(q, pr_fmt());
> +
> +	q = inet_frag_find(&net->nf_frag.frags, &key);
> +	if (!q)
>  		return NULL;
> -	}
> +
>  	return container_of(q, struct frag_queue, q);
>  }
>  
> @@ -593,8 +568,8 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user)
>  	fhdr = (struct frag_hdr *)skb_transport_header(skb);
>  
>  	skb_orphan(skb);
> -	fq = fq_find(net, fhdr->identification, user, &hdr->saddr, &hdr->daddr,
> -		     skb->dev ? skb->dev->ifindex : 0, ip6_frag_ecn(hdr));
> +	fq = fq_find(net, fhdr->identification, user, hdr,
> +		     skb->dev ? skb->dev->ifindex : 0);
>  	if (fq == NULL) {
>  		pr_debug("Can't find and can't create new queue\n");
>  		return -ENOMEM;
> @@ -656,17 +631,18 @@ static struct pernet_operations nf_ct_net_ops = {
>  	.exit = nf_ct_net_exit,
>  };
>  
> +extern const struct rhashtable_params ip6_rhash_params;

Can't we do this declaration in H-file?

> +
>  int nf_ct_frag6_init(void)
>  {
>  	int ret = 0;
>  
> -	nf_frags.hashfn = nf_hashfn;
>  	nf_frags.constructor = ip6_frag_init;
>  	nf_frags.destructor = NULL;
>  	nf_frags.qsize = sizeof(struct frag_queue);
> -	nf_frags.match = ip6_frag_match;
>  	nf_frags.frag_expire = nf_ct_frag6_expire;
>  	nf_frags.frags_cache_name = nf_frags_cache_name;
> +	nf_frags.rhash_params = ip6_rhash_params;
>  	ret = inet_frags_init(&nf_frags);
>  	if (ret)
>  		goto out;
> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> index 8cfea13a179c6f048177ac91fe26c8a5565e5820..737b0921ab0c9af198fefdf06d8f4ede91c7f3f6 100644
> --- a/net/ipv6/reassembly.c
> +++ b/net/ipv6/reassembly.c
> @@ -79,59 +79,19 @@ static struct inet_frags ip6_frags;
>  static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
>  			  struct net_device *dev);
>  
> -/*
> - * callers should be careful not to use the hash value outside the ipfrag_lock
> - * as doing so could race with ipfrag_hash_rnd being recalculated.
> - */
> -static unsigned int inet6_hash_frag(__be32 id, const struct in6_addr *saddr,
> -				    const struct in6_addr *daddr)
> -{
> -	net_get_random_once(&ip6_frags.rnd, sizeof(ip6_frags.rnd));
> -	return jhash_3words(ipv6_addr_hash(saddr), ipv6_addr_hash(daddr),
> -			    (__force u32)id, ip6_frags.rnd);
> -}
> -
> -static unsigned int ip6_hashfn(const struct inet_frag_queue *q)
> -{
> -	const struct frag_queue *fq;
> -
> -	fq = container_of(q, struct frag_queue, q);
> -	return inet6_hash_frag(fq->id, &fq->saddr, &fq->daddr);
> -}
> -
> -bool ip6_frag_match(const struct inet_frag_queue *q, const void *a)
> -{
> -	const struct frag_queue *fq;
> -	const struct ip6_create_arg *arg = a;
> -
> -	fq = container_of(q, struct frag_queue, q);
> -	return	fq->id == arg->id &&
> -		fq->user == arg->user &&
> -		ipv6_addr_equal(&fq->saddr, arg->src) &&
> -		ipv6_addr_equal(&fq->daddr, arg->dst) &&
> -		(arg->iif == fq->iif ||
> -		 !(ipv6_addr_type(arg->dst) & (IPV6_ADDR_MULTICAST |
> -					       IPV6_ADDR_LINKLOCAL)));
> -}
> -EXPORT_SYMBOL(ip6_frag_match);
> -
>  void ip6_frag_init(struct inet_frag_queue *q, const void *a)
>  {
>  	struct frag_queue *fq = container_of(q, struct frag_queue, q);
> -	const struct ip6_create_arg *arg = a;
> +	const struct frag_v6_compare_key *key = a;
>  
> -	fq->id = arg->id;
> -	fq->user = arg->user;
> -	fq->saddr = *arg->src;
> -	fq->daddr = *arg->dst;
> -	fq->ecn = arg->ecn;
> +	q->key.v6 = *key;
> +	fq->ecn = 0;
>  }
>  EXPORT_SYMBOL(ip6_frag_init);
>  
> -void ip6_expire_frag_queue(struct net *net, struct frag_queue *fq,
> -			   struct inet_frags *frags)
> +void ip6_expire_frag_queue(struct net *net, struct frag_queue *fq)
>  {
> -	struct net_device *dev = NULL;
> +	struct net_device *dev;
>  
>  	spin_lock(&fq->q.lock);
>  
> @@ -146,10 +106,6 @@ void ip6_expire_frag_queue(struct net *net, struct frag_queue *fq,
>  		goto out_rcu_unlock;
>  
>  	__IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMFAILS);
> -
> -	if (inet_frag_evicting(&fq->q))
> -		goto out_rcu_unlock;
> -
>  	__IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMTIMEOUT);
>  
>  	/* Don't send error if the first segment did not arrive. */
> @@ -179,31 +135,29 @@ static void ip6_frag_expire(struct timer_list *t)
>  	fq = container_of(frag, struct frag_queue, q);
>  	net = container_of(fq->q.net, struct net, ipv6.frags);
>  
> -	ip6_expire_frag_queue(net, fq, &ip6_frags);
> +	ip6_expire_frag_queue(net, fq);
>  }
>  
>  static struct frag_queue *
> -fq_find(struct net *net, __be32 id, const struct in6_addr *src,
> -	const struct in6_addr *dst, int iif, u8 ecn)
> +fq_find(struct net *net, __be32 id, const struct ipv6hdr *hdr, int iif)
>  {
> +	struct frag_v6_compare_key key = {
> +		.id = id,
> +		.saddr = hdr->saddr,
> +		.daddr = hdr->daddr,
> +		.user = IP6_DEFRAG_LOCAL_DELIVER,
> +		.iif = iif,
> +	};
>  	struct inet_frag_queue *q;
> -	struct ip6_create_arg arg;
> -	unsigned int hash;
>  
> -	arg.id = id;
> -	arg.user = IP6_DEFRAG_LOCAL_DELIVER;
> -	arg.src = src;
> -	arg.dst = dst;
> -	arg.iif = iif;
> -	arg.ecn = ecn;
> +	if (!(ipv6_addr_type(&hdr->daddr) & (IPV6_ADDR_MULTICAST |
> +					    IPV6_ADDR_LINKLOCAL)))
> +		key.iif = 0;
>  
> -	hash = inet6_hash_frag(id, src, dst);
> -
> -	q = inet_frag_find(&net->ipv6.frags, &ip6_frags, &arg, hash);
> -	if (IS_ERR_OR_NULL(q)) {
> -		inet_frag_maybe_warn_overflow(q, pr_fmt());
> +	q = inet_frag_find(&net->ipv6.frags, &key);
> +	if (!q)
>  		return NULL;
> -	}
> +
>  	return container_of(q, struct frag_queue, q);
>  }
>  
> @@ -527,10 +481,11 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *prev,
>  
>  static int ipv6_frag_rcv(struct sk_buff *skb)
>  {
> +	struct net *net = dev_net(skb_dst(skb)->dev);
> +	const struct ipv6hdr *hdr = ipv6_hdr(skb);
>  	struct frag_hdr *fhdr;
>  	struct frag_queue *fq;
> -	const struct ipv6hdr *hdr = ipv6_hdr(skb);
> -	struct net *net = dev_net(skb_dst(skb)->dev);
> +	int iif;
>  
>  	if (IP6CB(skb)->flags & IP6SKB_FRAGMENTED)
>  		goto fail_hdr;
> @@ -559,13 +514,14 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
>  		return 1;
>  	}
>  
> -	fq = fq_find(net, fhdr->identification, &hdr->saddr, &hdr->daddr,
> -		     skb->dev ? skb->dev->ifindex : 0, ip6_frag_ecn(hdr));
> +	iif = skb->dev ? skb->dev->ifindex : 0;
> +	fq = fq_find(net, fhdr->identification, hdr, iif);
>  	if (fq) {
>  		int ret;
>  
>  		spin_lock(&fq->q.lock);
>  
> +		fq->iif = iif;
>  		ret = ip6_frag_queue(fq, skb, fhdr, IP6CB(skb)->nhoff);
>  
>  		spin_unlock(&fq->q.lock);
> @@ -718,6 +674,7 @@ static int __net_init ipv6_frags_init_net(struct net *net)
>  	net->ipv6.frags.timeout = IPV6_FRAG_TIMEOUT;
>  	net->ipv6.frags.f = &ip6_frags;
>  
> +	net->ipv6.frags.f = &ip6_frags;

This '=' is already made above, we shouldn't do that twise...

>  	res = inet_frags_init_net(&net->ipv6.frags);
>  	if (res < 0)
>  		return res;
> @@ -739,14 +696,55 @@ static struct pernet_operations ip6_frags_ops = {
>  	.exit = ipv6_frags_exit_net,
>  };
>  
> +static u32 ip6_key_hashfn(const void *data, u32 len, u32 seed)
> +{
> +	return jhash2(data,
> +		      sizeof(struct frag_v6_compare_key) / sizeof(u32), seed);
> +}
> +
> +static u32 ip6_obj_hashfn(const void *data, u32 len, u32 seed)
> +{
> +	const struct inet_frag_queue *fq = data;
> +
> +	return jhash2((const u32 *)&fq->key.v6,
> +		      sizeof(struct frag_v6_compare_key) / sizeof(u32), seed);
> +}
> +
> +static int ip6_obj_cmpfn(struct rhashtable_compare_arg *arg, const void *ptr)
> +{
> +	const struct frag_v6_compare_key *key = arg->key;
> +	const struct inet_frag_queue *fq = ptr;
> +
> +	return !!memcmp(&fq->key, key, sizeof(*key));
> +}
> +
> +const struct rhashtable_params ip6_rhash_params = {
> +	.head_offset		= offsetof(struct inet_frag_queue, node),
> +	.hashfn			= ip6_key_hashfn,
> +	.obj_hashfn		= ip6_obj_hashfn,
> +	.obj_cmpfn		= ip6_obj_cmpfn,
> +	.automatic_shrinking	= true,
> +};
> +EXPORT_SYMBOL(ip6_rhash_params);
> +
>  int __init ipv6_frag_init(void)
>  {
>  	int ret;
>  
> -	ret = inet6_add_protocol(&frag_protocol, IPPROTO_FRAGMENT);
> +	ip6_frags.constructor = ip6_frag_init;
> +	ip6_frags.destructor = NULL;
> +	ip6_frags.qsize = sizeof(struct frag_queue);
> +	ip6_frags.frag_expire = ip6_frag_expire;
> +	ip6_frags.frags_cache_name = ip6_frag_cache_name;
> +	ip6_frags.rhash_params = ip6_rhash_params;
> +	ret = inet_frags_init(&ip6_frags);
>  	if (ret)
>  		goto out;
>  
> +	ret = inet6_add_protocol(&frag_protocol, IPPROTO_FRAGMENT);
> +	if (ret)
> +		goto err_protocol;
> +
>  	ret = ip6_frags_sysctl_register();
>  	if (ret)
>  		goto err_sysctl;
> @@ -755,16 +753,6 @@ int __init ipv6_frag_init(void)
>  	if (ret)
>  		goto err_pernet;
>  
> -	ip6_frags.hashfn = ip6_hashfn;
> -	ip6_frags.constructor = ip6_frag_init;
> -	ip6_frags.destructor = NULL;
> -	ip6_frags.qsize = sizeof(struct frag_queue);
> -	ip6_frags.match = ip6_frag_match;
> -	ip6_frags.frag_expire = ip6_frag_expire;
> -	ip6_frags.frags_cache_name = ip6_frag_cache_name;
> -	ret = inet_frags_init(&ip6_frags);
> -	if (ret)
> -		goto err_pernet;
>  out:
>  	return ret;
>  
> @@ -772,6 +760,8 @@ int __init ipv6_frag_init(void)
>  	ip6_frags_sysctl_unregister();
>  err_sysctl:
>  	inet6_del_protocol(&frag_protocol, IPPROTO_FRAGMENT);
> +err_protocol:
> +	inet_frags_fini(&ip6_frags);
>  	goto out;
>  }

Thanks,
Kirill

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox