[PATCH 0/1] ipset patch for nf

[PATCH 0/1] ipset patch for nf

[Jozsef Kadlecsik]

Hi Pablo,

Please apply the next patch against the nf tree:

- There was a race condition between parallel save/swap and delete,
  which resulted a kernel crash due to the increase ref for save, swap,
  wrong ref decrease operations. Reported and fixed by Vishwanath Pai.

The patch should be applied to the older stable kernel branches too.

Best regards,
Jozsef

The following changes since commit fe1c3e3f630ef7765f34d9585d6b524899502b3f:

  netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length (2016-03-16 21:43:50 +0100)

are available in the git repository at:

  git://blackhole.kfki.hu/nf master

for you to fetch changes up to 17f8a7334f3aee9783cfa6787172240c083ef394:

  netfilter: ipset: fix race condition in ipset save, swap and delete (2016-03-16 21:49:00 +0100)

----------------------------------------------------------------
Vishwanath Pai (1):
      netfilter: ipset: fix race condition in ipset save, swap and delete

 include/linux/netfilter/ipset/ip_set.h  |  4 ++++
 net/netfilter/ipset/ip_set_bitmap_gen.h |  2 +-
 net/netfilter/ipset/ip_set_core.c       | 33 ++++++++++++++++++++++++++++-----
 net/netfilter/ipset/ip_set_hash_gen.h   |  2 +-
 net/netfilter/ipset/ip_set_list_set.c   |  2 +-
 5 files changed, 35 insertions(+), 8 deletions(-)

[PATCH 1/1] netfilter: ipset: fix race condition in ipset save, swap and delete

[Jozsef Kadlecsik]

From: Vishwanath Pai <vpai@akamai.com>

This fix adds a new reference counter (ref_netlink) for the struct ip_set.
The other reference counter (ref) can be swapped out by ip_set_swap and we
need a separate counter to keep track of references for netlink events
like dump. Using the same ref counter for dump causes a race condition
which can be demonstrated by the following script:

ipset create hash_ip1 hash:ip family inet hashsize 1024 maxelem 500000 \
counters
ipset create hash_ip2 hash:ip family inet hashsize 300000 maxelem 500000 \
counters
ipset create hash_ip3 hash:ip family inet hashsize 1024 maxelem 500000 \
counters

ipset save &

ipset swap hash_ip3 hash_ip2
ipset destroy hash_ip3 /* will crash the machine */

Swap will exchange the values of ref so destroy will see ref = 0 instead of
ref = 1. With this fix in place swap will not succeed because ipset save
still has ref_netlink on the set (ip_set_swap doesn't swap ref_netlink).

Both delete and swap will error out if ref_netlink != 0 on the set.

Note: The changes to *_head functions is because previously we would
increment ref whenever we called these functions, we don't do that
anymore.

Reviewed-by: Joshua Hunt <johunt@akamai.com>
Signed-off-by: Vishwanath Pai <vpai@akamai.com>
Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
---
 include/linux/netfilter/ipset/ip_set.h  |  4 ++++
 net/netfilter/ipset/ip_set_bitmap_gen.h |  2 +-
 net/netfilter/ipset/ip_set_core.c       | 33 ++++++++++++++++++++++++++++-----
 net/netfilter/ipset/ip_set_hash_gen.h   |  2 +-
 net/netfilter/ipset/ip_set_list_set.c   |  2 +-
 5 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h
index 0e1f433..f48b8a6 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -234,6 +234,10 @@ struct ip_set {
 	spinlock_t lock;
 	/* References to the set */
 	u32 ref;
+	/* References to the set for netlink events like dump,
+	 * ref can be swapped out by ip_set_swap
+	 */
+	u32 ref_netlink;
 	/* The core set type */
 	struct ip_set_type *type;
 	/* The type variant doing the real job */
diff --git a/net/netfilter/ipset/ip_set_bitmap_gen.h b/net/netfilter/ipset/ip_set_bitmap_gen.h
index b0bc475..2e8e7e5 100644
--- a/net/netfilter/ipset/ip_set_bitmap_gen.h
+++ b/net/netfilter/ipset/ip_set_bitmap_gen.h
@@ -95,7 +95,7 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 	if (!nested)
 		goto nla_put_failure;
 	if (mtype_do_head(skb, map) ||
-	    nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref - 1)) ||
+	    nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref)) ||
 	    nla_put_net32(skb, IPSET_ATTR_MEMSIZE, htonl(memsize)))
 		goto nla_put_failure;
 	if (unlikely(ip_set_put_flags(skb, set)))
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 7e6568c..a748b0c 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -497,6 +497,26 @@ __ip_set_put(struct ip_set *set)
 	write_unlock_bh(&ip_set_ref_lock);
 }
 
+/* set->ref can be swapped out by ip_set_swap, netlink events (like dump) need
+ * a separate reference counter
+ */
+static inline void
+__ip_set_get_netlink(struct ip_set *set)
+{
+	write_lock_bh(&ip_set_ref_lock);
+	set->ref_netlink++;
+	write_unlock_bh(&ip_set_ref_lock);
+}
+
+static inline void
+__ip_set_put_netlink(struct ip_set *set)
+{
+	write_lock_bh(&ip_set_ref_lock);
+	BUG_ON(set->ref_netlink == 0);
+	set->ref_netlink--;
+	write_unlock_bh(&ip_set_ref_lock);
+}
+
 /* Add, del and test set entries from kernel.
  *
  * The set behind the index must exist and must be referenced
@@ -1002,7 +1022,7 @@ static int ip_set_destroy(struct net *net, struct sock *ctnl,
 	if (!attr[IPSET_ATTR_SETNAME]) {
 		for (i = 0; i < inst->ip_set_max; i++) {
 			s = ip_set(inst, i);
-			if (s && s->ref) {
+			if (s && (s->ref || s->ref_netlink)) {
 				ret = -IPSET_ERR_BUSY;
 				goto out;
 			}
@@ -1024,7 +1044,7 @@ static int ip_set_destroy(struct net *net, struct sock *ctnl,
 		if (!s) {
 			ret = -ENOENT;
 			goto out;
-		} else if (s->ref) {
+		} else if (s->ref || s->ref_netlink) {
 			ret = -IPSET_ERR_BUSY;
 			goto out;
 		}
@@ -1171,6 +1191,9 @@ static int ip_set_swap(struct net *net, struct sock *ctnl, struct sk_buff *skb,
 	      from->family == to->family))
 		return -IPSET_ERR_TYPE_MISMATCH;
 
+	if (from->ref_netlink || to->ref_netlink)
+		return -EBUSY;
+
 	strncpy(from_name, from->name, IPSET_MAXNAMELEN);
 	strncpy(from->name, to->name, IPSET_MAXNAMELEN);
 	strncpy(to->name, from_name, IPSET_MAXNAMELEN);
@@ -1206,7 +1229,7 @@ ip_set_dump_done(struct netlink_callback *cb)
 		if (set->variant->uref)
 			set->variant->uref(set, cb, false);
 		pr_debug("release set %s\n", set->name);
-		__ip_set_put_byindex(inst, index);
+		__ip_set_put_netlink(set);
 	}
 	return 0;
 }
@@ -1328,7 +1351,7 @@ dump_last:
 		if (!cb->args[IPSET_CB_ARG0]) {
 			/* Start listing: make sure set won't be destroyed */
 			pr_debug("reference set\n");
-			set->ref++;
+			set->ref_netlink++;
 		}
 		write_unlock_bh(&ip_set_ref_lock);
 		nlh = start_msg(skb, NETLINK_CB(cb->skb).portid,
@@ -1396,7 +1419,7 @@ release_refcount:
 		if (set->variant->uref)
 			set->variant->uref(set, cb, false);
 		pr_debug("release set %s\n", set->name);
-		__ip_set_put_byindex(inst, index);
+		__ip_set_put_netlink(set);
 		cb->args[IPSET_CB_ARG0] = 0;
 	}
 out:
diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index e5336ab..d32fd6b 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -1082,7 +1082,7 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 	if (nla_put_u32(skb, IPSET_ATTR_MARKMASK, h->markmask))
 		goto nla_put_failure;
 #endif
-	if (nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref - 1)) ||
+	if (nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref)) ||
 	    nla_put_net32(skb, IPSET_ATTR_MEMSIZE, htonl(memsize)))
 		goto nla_put_failure;
 	if (unlikely(ip_set_put_flags(skb, set)))
diff --git a/net/netfilter/ipset/ip_set_list_set.c b/net/netfilter/ipset/ip_set_list_set.c
index 24c6c19..a2a89e4 100644
--- a/net/netfilter/ipset/ip_set_list_set.c
+++ b/net/netfilter/ipset/ip_set_list_set.c
@@ -458,7 +458,7 @@ list_set_head(struct ip_set *set, struct sk_buff *skb)
 	if (!nested)
 		goto nla_put_failure;
 	if (nla_put_net32(skb, IPSET_ATTR_SIZE, htonl(map->size)) ||
-	    nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref - 1)) ||
+	    nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref)) ||
 	    nla_put_net32(skb, IPSET_ATTR_MEMSIZE,
 			  htonl(sizeof(*map) + n * set->dsize)))
 		goto nla_put_failure;
-- 
1.8.5.1

Re: [PATCH 0/1] ipset patch for nf

[Pablo Neira Ayuso]

On Wed, Mar 16, 2016 at 10:02:29PM +0100, Jozsef Kadlecsik wrote:
> Hi Pablo,
> 
> Please apply the next patch against the nf tree:
> 
> - There was a race condition between parallel save/swap and delete,
>   which resulted a kernel crash due to the increase ref for save, swap,
>   wrong ref decrease operations. Reported and fixed by Vishwanath Pai.
> 
> The patch should be applied to the older stable kernel branches too.
> 
> Best regards,
> Jozsef
> 
> The following changes since commit fe1c3e3f630ef7765f34d9585d6b524899502b3f:
> 
>   netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length (2016-03-16 21:43:50 +0100)
> 
> are available in the git repository at:
> 
>   git://blackhole.kfki.hu/nf master

Pulled, thanks Jozsef.

[PATCH 0/1] ipset patch for nf

[Jozsef Kadlecsik]

Hi Pablo,

Please consider to apply the next patch to the nf tree. It's larger than
usual, but the issue could not be solved simpler.

- Fix "INFO: rcu detected stall in hash_xxx" reports of syzbot
  by introducing region locking and using workqueue instead of timer based
  gc of timed out entries in hash types of sets in ipset.

Best regards,
Jozsef

The following changes since commit 83d0585f91da441a0b11bc5ff93f4cda56de6703:

  Merge branch 'Fix-reconnection-latency-caused-by-FIN-ACK-handling-race' (2020-02-02 13:45:05 -0800)

are available in the Git repository at:

  git://blackhole.kfki.hu/nf f431c76a1f36bcd6bbfd

for you to fetch changes up to f431c76a1f36bcd6bbfd668a98127ac60226b86f:

  netfilter: ipset: Fix "INFO: rcu detected stall in hash_xxx" reports (2020-02-18 11:19:06 +0100)

----------------------------------------------------------------
Jozsef Kadlecsik (1):
      netfilter: ipset: Fix "INFO: rcu detected stall in hash_xxx" reports

 include/linux/netfilter/ipset/ip_set.h |  11 +-
 net/netfilter/ipset/ip_set_core.c      |  34 +-
 net/netfilter/ipset/ip_set_hash_gen.h  | 633 +++++++++++++++++++++++----------
 3 files changed, 472 insertions(+), 206 deletions(-)

[PATCH 0/1] ipset patch for nf

[Jozsef Kadlecsik]

Hi Pablo,

Please apply the next patch for the nf tree. It fixes the issue
when an IP range with more than 2^31 addresses was specified to
add (bugzilla id #1005)

- Fix adding an IPv4 range containing more than 2^31 addresses,
  which failed silently due to wrong comparison (bugzilla id #1005).

Thanks,
Jozsef
-

The following changes since commit 7f4f7dd4417d9efd038b14d39c70170db2e0baa0:

  netfilter: ipset: ipset list may return wrong member count for set with timeout (2017-09-18 17:35:32 +0200)

are available in the git repository at:

  git://blackhole.kfki.hu/nf 

for you to fetch changes up to 625a0411a81ae51c12bde4452171f08174b656cd:

  Fix adding an IPv4 range containing more than 2^31 addresses (2017-09-23 23:28:13 +0200)

----------------------------------------------------------------
Jozsef Kadlecsik (1):
      Fix adding an IPv4 range containing more than 2^31 addresses

 net/netfilter/ipset/ip_set_hash_ip.c         | 22 ++++++++++++----------
 net/netfilter/ipset/ip_set_hash_ipmark.c     |  2 +-
 net/netfilter/ipset/ip_set_hash_ipport.c     |  2 +-
 net/netfilter/ipset/ip_set_hash_ipportip.c   |  2 +-
 net/netfilter/ipset/ip_set_hash_ipportnet.c  |  4 ++--
 net/netfilter/ipset/ip_set_hash_net.c        |  2 +-
 net/netfilter/ipset/ip_set_hash_netiface.c   |  2 +-
 net/netfilter/ipset/ip_set_hash_netnet.c     |  4 ++--
 net/netfilter/ipset/ip_set_hash_netport.c    |  2 +-
 net/netfilter/ipset/ip_set_hash_netportnet.c |  4 ++--
 10 files changed, 24 insertions(+), 22 deletions(-)

Re: [PATCH 0/1] ipset patch for nf

[Pablo Neira Ayuso]

On Tue, Mar 08, 2016 at 08:44:19PM +0100, Jozsef Kadlecsik wrote:
> Hi Pablo,
> 
> Please apply the next patch against the nf tree:
> 
> - Julia Lawall pointed out that IPSET_ATTR_ETHER netlink attribute
>   length was not checked explicitly. The patch adds the missing
>   checkings.
> 
> The patch should be applied to the older stable kernel branches too.
> 
> Best regards,
> Jozsef
> 
> The following changes since commit 45040978c8994d1401baf5cc5ac71c1495d4e120:
> 
>   netfilter: ipset: Fix set:list type crash when flush/dump set in parallel (2016-02-24 20:32:21 +0100)
> 
> are available in the git repository at:
> 
>   git://blackhole.kfki.hu/nf master

This patch also came with the previous pull, so this will show up in
the nf-next tree too.

Thanks.

[PATCH 0/1] ipset patch for nf

[Jozsef Kadlecsik]

Hi Pablo,

Please apply the next patch against the nf tree:

- Julia Lawall pointed out that IPSET_ATTR_ETHER netlink attribute
  length was not checked explicitly. The patch adds the missing
  checkings.

The patch should be applied to the older stable kernel branches too.

Best regards,
Jozsef

The following changes since commit 45040978c8994d1401baf5cc5ac71c1495d4e120:

  netfilter: ipset: Fix set:list type crash when flush/dump set in parallel (2016-02-24 20:32:21 +0100)

are available in the git repository at:

  git://blackhole.kfki.hu/nf master

for you to fetch changes up to d8aacd87180141ff6b812b53de77a4336e87c91a:

  netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length (2016-03-08 20:36:17 +0100)

----------------------------------------------------------------
Jozsef Kadlecsik (1):
      netfilter: ipset: Check IPSET_ATTR_ETHER netlink attribute length

 net/netfilter/ipset/ip_set_bitmap_ipmac.c | 2 ++
 net/netfilter/ipset/ip_set_hash_mac.c     | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

[PATCH 0/1] ipset patch for nf

[Jozsef Kadlecsik]

Hi Pablo,

Please apply the next patch against the nf tree:

- Deniz Eren reported that parallel flush/dump of list:set type of sets
  can lead to kernel crash. The bug was due to non-RCU compatible flushing,
  listing in the set type, fixed by me.

The following changes since commit 5cc6ce9ff27565949a1001a2889a8dd9fd09e772:

  netfilter: nft_counter: fix erroneous return values (2016-02-08 13:05:02 +0100)

are available in the git repository at:

  git://blackhole.kfki.hu/nf master

for you to fetch changes up to 45040978c8994d1401baf5cc5ac71c1495d4e120:

  netfilter: ipset: Fix set:list type crash when flush/dump set in parallel (2016-02-24 20:32:21 +0100)

----------------------------------------------------------------
Jozsef Kadlecsik (1):
      netfilter: ipset: Fix set:list type crash when flush/dump set in parallel

 net/netfilter/ipset/ip_set_core.c     |  3 +++
 net/netfilter/ipset/ip_set_list_set.c | 55 ++++++++++++++++++++++--------------------------
 2 files changed, 28 insertions(+), 30 deletions(-)

Re: [PATCH 0/1] ipset patch for nf

[Pablo Neira Ayuso]

On Fri, Aug 28, 2015 at 07:15:51PM +0200, Jozsef Kadlecsik wrote:
> Hi Pablo,
> 
> Please apply the next bugfix patch against the nf tree.
> 
> - Dave Jones reported that KASan detected out of bounds access in hash:net*
>   types, which is fixed in the next patch
> 
> Thanks,
> Jozsef
> 
> The following changes since commit 18e1db67e93ed75d9dc0d34c8d783ccf10547c2b:
> 
>   netfilter: bridge: fix IPv6 packets not being bridged with CONFIG_IPV6=n (2015-08-19 21:21:41 +0200)
> 
> are available in the git repository at:
> 
>   git://blackhole.kfki.hu/nf master

Pulled into nf, thanks Jozsef.

[PATCH 0/1] ipset patch for nf

[Jozsef Kadlecsik]

Hi Pablo,

Please apply the next bugfix patch against the nf tree.

- Dave Jones reported that KASan detected out of bounds access in hash:net*
  types, which is fixed in the next patch

Thanks,
Jozsef

The following changes since commit 18e1db67e93ed75d9dc0d34c8d783ccf10547c2b:

  netfilter: bridge: fix IPv6 packets not being bridged with CONFIG_IPV6=n (2015-08-19 21:21:41 +0200)

are available in the git repository at:

  git://blackhole.kfki.hu/nf master

for you to fetch changes up to 6fe7ccfd77415a6ba250c10c580eb3f9acf79753:

  netfilter: ipset: Out of bound access in hash:net* types fixed (2015-08-28 18:51:30 +0200)

----------------------------------------------------------------
Jozsef Kadlecsik (1):
      netfilter: ipset: Out of bound access in hash:net* types fixed

 net/netfilter/ipset/ip_set_hash_gen.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
kadlec@blackhole ~/git/nf $