* [PATCH net 1/3] netfilter: ipset: regression in ip_set_hash_ip.c
2022-11-22 21:28 [PATCH net 0/3] Netfilter fixes for net Pablo Neira Ayuso
@ 2022-11-22 21:28 ` Pablo Neira Ayuso
2022-11-24 3:20 ` patchwork-bot+netdevbpf
2022-11-22 21:28 ` [PATCH net 2/3] netfilter: ipset: restore allowing 64 clashing elements in hash:net,iface Pablo Neira Ayuso
2022-11-22 21:28 ` [PATCH net 3/3] netfilter: flowtable_offload: add missing locking Pablo Neira Ayuso
2 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2022-11-22 21:28 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet
From: Vishwanath Pai <vpai@akamai.com>
This patch introduced a regression: commit 48596a8ddc46 ("netfilter:
ipset: Fix adding an IPv4 range containing more than 2^31 addresses")
The variable e.ip is passed to adtfn() function which finally adds the
ip address to the set. The patch above refactored the for loop and moved
e.ip = htonl(ip) to the end of the for loop.
What this means is that if the value of "ip" changes between the first
assignement of e.ip and the forloop, then e.ip is pointing to a
different ip address than "ip".
Test case:
$ ipset create jdtest_tmp hash:ip family inet hashsize 2048 maxelem 100000
$ ipset add jdtest_tmp 10.0.1.1/31
ipset v6.21.1: Element cannot be added to the set: it's already added
The value of ip gets updated inside the "else if (tb[IPSET_ATTR_CIDR])"
block but e.ip is still pointing to the old value.
Fixes: 48596a8ddc46 ("netfilter: ipset: Fix adding an IPv4 range containing more than 2^31 addresses")
Reviewed-by: Joshua Hunt <johunt@akamai.com>
Signed-off-by: Vishwanath Pai <vpai@akamai.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/ipset/ip_set_hash_ip.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/ipset/ip_set_hash_ip.c b/net/netfilter/ipset/ip_set_hash_ip.c
index dd30c03d5a23..75d556d71652 100644
--- a/net/netfilter/ipset/ip_set_hash_ip.c
+++ b/net/netfilter/ipset/ip_set_hash_ip.c
@@ -151,18 +151,16 @@ hash_ip4_uadt(struct ip_set *set, struct nlattr *tb[],
if (((u64)ip_to - ip + 1) >> (32 - h->netmask) > IPSET_MAX_RANGE)
return -ERANGE;
- if (retried) {
+ if (retried)
ip = ntohl(h->next.ip);
- e.ip = htonl(ip);
- }
for (; ip <= ip_to;) {
+ e.ip = htonl(ip);
ret = adtfn(set, &e, &ext, &ext, flags);
if (ret && !ip_set_eexist(ret, flags))
return ret;
ip += hosts;
- e.ip = htonl(ip);
- if (e.ip == 0)
+ if (ip == 0)
return 0;
ret = 0;
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net 1/3] netfilter: ipset: regression in ip_set_hash_ip.c
2022-11-22 21:28 ` [PATCH net 1/3] netfilter: ipset: regression in ip_set_hash_ip.c Pablo Neira Ayuso
@ 2022-11-24 3:20 ` patchwork-bot+netdevbpf
0 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-11-24 3:20 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, davem, netdev, kuba, pabeni, edumazet
Hello:
This series was applied to netdev/net.git (master)
by Pablo Neira Ayuso <pablo@netfilter.org>:
On Tue, 22 Nov 2022 22:28:12 +0100 you wrote:
> From: Vishwanath Pai <vpai@akamai.com>
>
> This patch introduced a regression: commit 48596a8ddc46 ("netfilter:
> ipset: Fix adding an IPv4 range containing more than 2^31 addresses")
>
> The variable e.ip is passed to adtfn() function which finally adds the
> ip address to the set. The patch above refactored the for loop and moved
> e.ip = htonl(ip) to the end of the for loop.
>
> [...]
Here is the summary with links:
- [net,1/3] netfilter: ipset: regression in ip_set_hash_ip.c
https://git.kernel.org/netdev/net/c/c7aa1a76d4a0
- [net,2/3] netfilter: ipset: restore allowing 64 clashing elements in hash:net,iface
https://git.kernel.org/netdev/net/c/6a66ce44a51b
- [net,3/3] netfilter: flowtable_offload: add missing locking
https://git.kernel.org/netdev/net/c/bcd9e3c1656d
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net 2/3] netfilter: ipset: restore allowing 64 clashing elements in hash:net,iface
2022-11-22 21:28 [PATCH net 0/3] Netfilter fixes for net Pablo Neira Ayuso
2022-11-22 21:28 ` [PATCH net 1/3] netfilter: ipset: regression in ip_set_hash_ip.c Pablo Neira Ayuso
@ 2022-11-22 21:28 ` Pablo Neira Ayuso
2022-11-22 21:28 ` [PATCH net 3/3] netfilter: flowtable_offload: add missing locking Pablo Neira Ayuso
2 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2022-11-22 21:28 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet
From: Jozsef Kadlecsik <kadlec@netfilter.org>
The commit 510841da1fcc ("netfilter: ipset: enforce documented limit to
prevent allocating huge memory") was too strict and prevented to add up to
64 clashing elements to a hash:net,iface type of set. This patch fixes the
issue and now the type behaves as documented.
Fixes: 510841da1fcc ("netfilter: ipset: enforce documented limit to prevent allocating huge memory")
Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/ipset/ip_set_hash_gen.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index 3adc291d9ce1..7499192af586 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -916,7 +916,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
#ifdef IP_SET_HASH_WITH_MULTI
if (h->bucketsize >= AHASH_MAX_TUNED)
goto set_full;
- else if (h->bucketsize < multi)
+ else if (h->bucketsize <= multi)
h->bucketsize += AHASH_INIT_SIZE;
#endif
if (n->size >= AHASH_MAX(h)) {
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net 3/3] netfilter: flowtable_offload: add missing locking
2022-11-22 21:28 [PATCH net 0/3] Netfilter fixes for net Pablo Neira Ayuso
2022-11-22 21:28 ` [PATCH net 1/3] netfilter: ipset: regression in ip_set_hash_ip.c Pablo Neira Ayuso
2022-11-22 21:28 ` [PATCH net 2/3] netfilter: ipset: restore allowing 64 clashing elements in hash:net,iface Pablo Neira Ayuso
@ 2022-11-22 21:28 ` Pablo Neira Ayuso
2 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2022-11-22 21:28 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev, kuba, pabeni, edumazet
From: Felix Fietkau <nbd@nbd.name>
nf_flow_table_block_setup and the driver TC_SETUP_FT call can modify the flow
block cb list while they are being traversed elsewhere, causing a crash.
Add a write lock around the calls to protect readers
Fixes: c29f74e0df7a ("netfilter: nf_flow_table: hardware offload support")
Reported-by: Chad Monroe <chad.monroe@smartrg.com>
Signed-off-by: Felix Fietkau <nbd@nbd.name>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_flow_table_offload.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index b04645ced89b..00b522890d77 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -1098,6 +1098,7 @@ static int nf_flow_table_block_setup(struct nf_flowtable *flowtable,
struct flow_block_cb *block_cb, *next;
int err = 0;
+ down_write(&flowtable->flow_block_lock);
switch (cmd) {
case FLOW_BLOCK_BIND:
list_splice(&bo->cb_list, &flowtable->flow_block.cb_list);
@@ -1112,6 +1113,7 @@ static int nf_flow_table_block_setup(struct nf_flowtable *flowtable,
WARN_ON_ONCE(1);
err = -EOPNOTSUPP;
}
+ up_write(&flowtable->flow_block_lock);
return err;
}
@@ -1168,7 +1170,9 @@ static int nf_flow_table_offload_cmd(struct flow_block_offload *bo,
nf_flow_table_block_offload_init(bo, dev_net(dev), cmd, flowtable,
extack);
+ down_write(&flowtable->flow_block_lock);
err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_FT, bo);
+ up_write(&flowtable->flow_block_lock);
if (err < 0)
return err;
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread