[PATCH 0/1] ipset patch for nf tree

[PATCH 0/1] ipset patch for nf tree

[Jozsef Kadlecsik]

Hi Pablo,

Please apply the next patch against your nf tree so that it'll get 
applied to older stable branches too.

- Kyle Zeng reported that there is a race between IPSET_CMD_ADD and IPSET_CMD_SWAP:
  when the schedule point was added to call_ad(), the wrong reference counter was
  used. For long taking operations initiated from userspace the ref_netlink reference
  counter must be used to exclude concurrent clashing operations.

Best regards,
Jozsef

The following changes since commit 7153a404fb70d21097af3169354e1e5fda3fbb02:

  Merge tag 'nf-23-09-06' of https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf (2023-09-07 11:47:15 +0200)

are available in the Git repository at:

  git://blackhole.kfki.hu/nf 5adf434ae86e34a0c

for you to fetch changes up to 5adf434ae86e34a0cff2fd0aa737dab16d7f4812:

  netfilter: ipset: Fix race between IPSET_CMD_CREATE and IPSET_CMD_SWAP (2023-09-19 12:34:45 +0200)

----------------------------------------------------------------
Jozsef Kadlecsik (1):
      netfilter: ipset: Fix race between IPSET_CMD_CREATE and IPSET_CMD_SWAP

 net/netfilter/ipset/ip_set_core.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

[PATCH 1/1] netfilter: ipset: Fix race between IPSET_CMD_CREATE and IPSET_CMD_SWAP

[Jozsef Kadlecsik]

Kyle Zeng reported that there is a race between IPSET_CMD_ADD and IPSET_CMD_SWAP
in netfilter/ip_set, which can lead to the invocation of `__ip_set_put` on a wrong
`set`, triggering the `BUG_ON(set->ref == 0);` check in it.

The race is caused by using the wrong reference counter, i.e. the ref counter instead
of ref_netlink.

Fixes: 24e227896bbf ("netfilter: ipset: Add schedule point in call_ad().")
Reported-by: Kyle Zeng <zengyhkyle@gmail.com>
Tested-by: Kyle Zeng <zengyhkyle@gmail.com>
Signed-off-by: Jozsef Kadlecsik <kadlec@netfilter.org>
---
 net/netfilter/ipset/ip_set_core.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index e564b5174261..35d2f9c9ada0 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -682,6 +682,14 @@ __ip_set_put(struct ip_set *set)
 /* set->ref can be swapped out by ip_set_swap, netlink events (like dump) need
  * a separate reference counter
  */
+static 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 void
 __ip_set_put_netlink(struct ip_set *set)
 {
@@ -1693,11 +1701,11 @@ call_ad(struct net *net, struct sock *ctnl, struct sk_buff *skb,
 
 	do {
 		if (retried) {
-			__ip_set_get(set);
+			__ip_set_get_netlink(set);
 			nfnl_unlock(NFNL_SUBSYS_IPSET);
 			cond_resched();
 			nfnl_lock(NFNL_SUBSYS_IPSET);
-			__ip_set_put(set);
+			__ip_set_put_netlink(set);
 		}
 
 		ip_set_lock(set);
-- 
2.30.2

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

[Pablo Neira Ayuso]

Hi Jozsef,

On Tue, Sep 19, 2023 at 08:04:44PM +0200, Jozsef Kadlecsik wrote:
> Hi Pablo,
> 
> Please apply the next patch against your nf tree so that it'll get 
> applied to older stable branches too.
> 
> - Kyle Zeng reported that there is a race between IPSET_CMD_ADD and IPSET_CMD_SWAP:
>   when the schedule point was added to call_ad(), the wrong reference counter was
>   used. For long taking operations initiated from userspace the ref_netlink reference
>   counter must be used to exclude concurrent clashing operations.

Thanks.

Florian is taking care of this round of nf.git fixes, he will be
collecting this by tomorrow.