[{"id":3677583,"web_url":"http://patchwork.ozlabs.org/comment/3677583/","msgid":"","list_archive_url":null,"date":"2026-04-15T10:22:05","subject":"Re: [PATCH 2/2] netfilter: ipset: Fix data race between add and dump\n in all hash types","submitter":{"id":1025,"url":"http://patchwork.ozlabs.org/api/people/1025/","name":"Florian Westphal","email":"fw@strlen.de"},"content":"Jozsef Kadlecsik wrote:\n> When adding a new entry to the next position in the existing hash bucket,\n> the position index was incremented too early and parallel dump could\n> read it before the entry was populated with the value. Move the setting\n> of the position index after populating the entry.\n> \n> Reported-by: syzbot+786c889f046e8b003ca6@syzkaller.appspotmail.com\n> Reported-by: syzbot+1da17e4b41d795df059e@syzkaller.appspotmail.com\n> Reported-by: syzbot+421c5f3ff8e9493084d9@syzkaller.appspotmail.com\n> Signed-off-by: Jozsef Kadlecsik \n> ---\n> net/netfilter/ipset/ip_set_hash_gen.h | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n> \n> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h\n> index b79e5dd2af03..0da02a8dfbae 100644\n> --- a/net/netfilter/ipset/ip_set_hash_gen.h\n> +++ b/net/netfilter/ipset/ip_set_hash_gen.h\n> @@ -844,7 +844,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n> \tconst struct mtype_elem *d = value;\n> \tstruct mtype_elem *data;\n> \tstruct hbucket *n, *old = ERR_PTR(-ENOENT);\n> -\tint i, j = -1, ret;\n> +\tint i, j = -1, npos = 0, ret;\n> \tbool flag_exist = flags & IPSET_FLAG_EXIST;\n> \tbool deleted = false, forceadd = false, reuse = false;\n> \tu32 r, key, multi = 0, elements, maxelem;\n> @@ -889,6 +889,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n> \t\t\text_size(AHASH_INIT_SIZE, set->dsize);\n> \t\tgoto copy_elem;\n> \t}\n> +\tnpos = n->pos;\n> \tfor (i = 0; i < n->pos; i++) {\n> \t\tif (!test_bit(i, n->used)) {\n> \t\t\t/* Reuse first deleted entry */\n> @@ -962,7 +963,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n> \t}\n> \n> copy_elem:\n> -\tj = n->pos++;\n> +\tj = npos = n->pos + 1;\n\nHmm. Should that be:\n+\tj = npos;\n+\tnpos = n->pos + 1;\n\nAs-is j is advanced by 1 compared to old code.\n\n> \tdata = ahash_data(n, j, set->dsize);\n> copy_data:\n> \tt->hregion[r].elements++;\n> @@ -985,6 +986,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n> \tif (SET_WITH_TIMEOUT(set))\n> \t\tip_set_timeout_set(ext_timeout(data, set), ext->timeout);\n> \tsmp_mb__before_atomic();\n> +\tn->pos = npos;\n> \tset_bit(j, n->used);\n\nI think this needs a followup-patch to switch this to smp_store_release\nand readers to smp_load_acquire helpers.\n\nHere is a diff for this (generated by LLM and only compile tested).\nI think this can be a separate patch to not make this change too big and\nto not mix different fixes.\n\ndiff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h\nindex 0da02a8dfbae..2ca674e51699 100644\n--- a/net/netfilter/ipset/ip_set_hash_gen.h\n+++ b/net/netfilter/ipset/ip_set_hash_gen.h\n@@ -682,10 +682,13 @@ mtype_resize(struct ip_set *set, bool retried)\n \t\trcu_read_lock_bh();\n \t\tfor (i = ahash_bucket_start(r, orig->htable_bits);\n \t\t i < ahash_bucket_end(r, orig->htable_bits); i++) {\n+\t\t\tu8 pos;\n+\n \t\t\tn = __ipset_dereference(hbucket(orig, i));\n \t\t\tif (!n)\n \t\t\t\tcontinue;\n-\t\t\tfor (j = 0; j < n->pos; j++) {\n+\t\t\tpos = smp_load_acquire(&n->pos);\n+\t\t\tfor (j = 0; j < pos; j++) {\n \t\t\t\tif (!test_bit(j, n->used))\n \t\t\t\t\tcontinue;\n \t\t\t\tdata = ahash_data(n, j, dsize);\n@@ -817,10 +820,13 @@ mtype_ext_size(struct ip_set *set, u32 *elements, size_t *ext_size)\n \tfor (r = 0; r < ahash_numof_locks(t->htable_bits); r++) {\n \t\tfor (i = ahash_bucket_start(r, t->htable_bits);\n \t\t i < ahash_bucket_end(r, t->htable_bits); i++) {\n+\t\t\tu8 pos;\n+\n \t\t\tn = rcu_dereference_bh(hbucket(t, i));\n \t\t\tif (!n)\n \t\t\t\tcontinue;\n-\t\t\tfor (j = 0; j < n->pos; j++) {\n+\t\t\tpos = smp_load_acquire(&n->pos);\n+\t\t\tfor (j = 0; j < pos; j++) {\n \t\t\t\tif (!test_bit(j, n->used))\n \t\t\t\t\tcontinue;\n \t\t\t\tdata = ahash_data(n, j, set->dsize);\n@@ -963,7 +969,8 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n \t}\n \n copy_elem:\n-\tj = npos = n->pos + 1;\n+\tj = npos;\n+\tnpos = n->pos + 1;\n \tdata = ahash_data(n, j, set->dsize);\n copy_data:\n \tt->hregion[r].elements++;\n@@ -985,8 +992,8 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n \t/* Must come last for the case when timed out entry is reused */\n \tif (SET_WITH_TIMEOUT(set))\n \t\tip_set_timeout_set(ext_timeout(data, set), ext->timeout);\n-\tsmp_mb__before_atomic();\n-\tn->pos = npos;\n+\t/* Ensure all data writes are visible before updating position */\n+\tsmp_store_release(&n->pos, npos);\n \tset_bit(j, n->used);\n \tif (old != ERR_PTR(-ENOENT)) {\n \t\trcu_assign_pointer(hbucket(t, key), n);\n@@ -1172,6 +1179,7 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,\n \tint ret, i, j = 0;\n #endif\n \tu32 key, multi = 0;\n+\tu8 pos;\n \n \tpr_debug(\"test by nets\\n\");\n \tfor (; j < NLEN && h->nets[j].cidr[0] && !multi; j++) {\n@@ -1189,7 +1197,8 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,\n \t\tn = rcu_dereference_bh(hbucket(t, key));\n \t\tif (!n)\n \t\t\tcontinue;\n-\t\tfor (i = 0; i < n->pos; i++) {\n+\t\tpos = smp_load_acquire(&n->pos);\n+\t\tfor (i = 0; i < pos; i++) {\n \t\t\tif (!test_bit(i, n->used))\n \t\t\t\tcontinue;\n \t\t\tdata = ahash_data(n, i, set->dsize);\n@@ -1223,6 +1232,7 @@ mtype_test(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n \tstruct mtype_elem *data;\n \tint i, ret = 0;\n \tu32 key, multi = 0;\n+\tu8 pos;\n \n \trcu_read_lock_bh();\n \tt = rcu_dereference_bh(h->table);\n@@ -1245,7 +1255,8 @@ mtype_test(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n \t\tret = 0;\n \t\tgoto out;\n \t}\n-\tfor (i = 0; i < n->pos; i++) {\n+\tpos = smp_load_acquire(&n->pos);\n+\tfor (i = 0; i < pos; i++) {\n \t\tif (!test_bit(i, n->used))\n \t\t\tcontinue;\n \t\tdata = ahash_data(n, i, set->dsize);\n@@ -1373,6 +1384,8 @@ mtype_list(const struct ip_set *set,\n \trcu_read_lock();\n \tfor (; cb->args[IPSET_CB_ARG0] < jhash_size(t->htable_bits);\n \t cb->args[IPSET_CB_ARG0]++) {\n+\t\tu8 pos;\n+\n \t\tcond_resched_rcu();\n \t\tincomplete = skb_tail_pointer(skb);\n \t\tn = rcu_dereference(hbucket(t, cb->args[IPSET_CB_ARG0]));\n@@ -1380,7 +1393,9 @@ mtype_list(const struct ip_set *set,\n \t\t\t cb->args[IPSET_CB_ARG0], t, n);\n \t\tif (!n)\n \t\t\tcontinue;\n-\t\tfor (i = 0; i < n->pos; i++) {\n+\t\t/* Acquire ordering with smp_store_release in mtype_add */\n+\t\tpos = smp_load_acquire(&n->pos);\n+\t\tfor (i = 0; i < pos; i++) {\n \t\t\tif (!test_bit(i, n->used))\n \t\t\t\tcontinue;\n \t\t\te = ahash_data(n, i, set->dsize);","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","netfilter-devel@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c09:e001:a7::12fc:5321; helo=sto.lore.kernel.org;\n envelope-from=netfilter-devel+bounces-11910-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=91.216.245.30","smtp.subspace.kernel.org;\n dmarc=none (p=none dis=none) header.from=strlen.de","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=strlen.de"],"Received":["from sto.lore.kernel.org (sto.lore.kernel.org\n [IPv6:2600:3c09:e001:a7::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fwcdg679fz1yCv\n\tfor ; Wed, 15 Apr 2026 20:22:15 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sto.lore.kernel.org (Postfix) with ESMTP id 34214302541A\n\tfor ; Wed, 15 Apr 2026 10:22:12 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 476E733FE1F;\n\tWed, 15 Apr 2026 10:22:10 +0000 (UTC)","from Chamillionaire.breakpoint.cc (Chamillionaire.breakpoint.cc\n [91.216.245.30])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id C96B5337BB5\n\tfor ; Wed, 15 Apr 2026 10:22:07 +0000 (UTC)","by Chamillionaire.breakpoint.cc (Postfix, from userid 1003)\n\tid EE0FF6066A; Wed, 15 Apr 2026 12:22:05 +0200 (CEST)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776248530; cv=none;\n b=gD/xljW5GxrATiulBII8Jnn1R8DofPcuUso2TqAwbzEbnjQo0iBU81xw6v6oodev98rt7ZVma46Yc+aEQzaT6P3VEwgy3krhFmBK1nED9ga6wxxjvyl8CsWamjTDhMpDsUWqmLfP47vJygOxlbzY8SaGM+xf+s7pNAnDVjV6YzI=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776248530; c=relaxed/simple;\n\tbh=qutAotB3W84SBojsd0uqK5ZPCD4ZssnMBA2Fo3MzLW0=;\n\th=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:\n\t Content-Type:Content-Disposition:In-Reply-To;\n b=KwixQNxCNbIwlrFAePclxADz2r6uLTTn1MA8MVnLQMa1HHAOBuCRDMPyXy+KTBBwY0h53p8XxRq+ElnHLgIZt8FcOtJGeKSwY5BbLA1w9zbdkMFFM9u1J4QtVyOR7QNEMpwFDnaHrzr++N15FKy+CbzaeLM8AppuIoZg3pNMWnk=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=none (p=none dis=none) header.from=strlen.de;\n spf=pass smtp.mailfrom=strlen.de; arc=none smtp.client-ip=91.216.245.30","Date":"Wed, 15 Apr 2026 12:22:05 +0200","From":"Florian Westphal ","To":"Jozsef Kadlecsik ","Cc":"netfilter-devel@vger.kernel.org,\n\tPablo Neira Ayuso ","Subject":"Re: [PATCH 2/2] netfilter: ipset: Fix data race between add and dump\n in all hash types","Message-ID":"","References":"<20260415082039.4133308-1-kadlec@netfilter.org>\n <20260415082039.4133308-3-kadlec@netfilter.org>","Precedence":"bulk","X-Mailing-List":"netfilter-devel@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20260415082039.4133308-3-kadlec@netfilter.org>"}},{"id":3677587,"web_url":"http://patchwork.ozlabs.org/comment/3677587/","msgid":"<93dd963e-7577-83a5-244f-f19f49ddcd78@blackhole.kfki.hu>","list_archive_url":null,"date":"2026-04-15T10:33:36","subject":"Re: [PATCH 2/2] netfilter: ipset: Fix data race between add and dump\n in all hash types","submitter":{"id":1575,"url":"http://patchwork.ozlabs.org/api/people/1575/","name":"Jozsef Kadlecsik","email":"kadlec@blackhole.kfki.hu"},"content":"Hi Florian,\n\nOn Wed, 15 Apr 2026, Florian Westphal wrote:\n\n> Jozsef Kadlecsik wrote:\n>> When adding a new entry to the next position in the existing hash bucket,\n>> the position index was incremented too early and parallel dump could\n>> read it before the entry was populated with the value. Move the setting\n>> of the position index after populating the entry.\n>>\n>> Reported-by: syzbot+786c889f046e8b003ca6@syzkaller.appspotmail.com\n>> Reported-by: syzbot+1da17e4b41d795df059e@syzkaller.appspotmail.com\n>> Reported-by: syzbot+421c5f3ff8e9493084d9@syzkaller.appspotmail.com\n>> Signed-off-by: Jozsef Kadlecsik \n>> ---\n>> net/netfilter/ipset/ip_set_hash_gen.h | 6 ++++--\n>> 1 file changed, 4 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h\n>> index b79e5dd2af03..0da02a8dfbae 100644\n>> --- a/net/netfilter/ipset/ip_set_hash_gen.h\n>> +++ b/net/netfilter/ipset/ip_set_hash_gen.h\n>> @@ -844,7 +844,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n>> \tconst struct mtype_elem *d = value;\n>> \tstruct mtype_elem *data;\n>> \tstruct hbucket *n, *old = ERR_PTR(-ENOENT);\n>> -\tint i, j = -1, ret;\n>> +\tint i, j = -1, npos = 0, ret;\n>> \tbool flag_exist = flags & IPSET_FLAG_EXIST;\n>> \tbool deleted = false, forceadd = false, reuse = false;\n>> \tu32 r, key, multi = 0, elements, maxelem;\n>> @@ -889,6 +889,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n>> \t\t\text_size(AHASH_INIT_SIZE, set->dsize);\n>> \t\tgoto copy_elem;\n>> \t}\n>> +\tnpos = n->pos;\n>> \tfor (i = 0; i < n->pos; i++) {\n>> \t\tif (!test_bit(i, n->used)) {\n>> \t\t\t/* Reuse first deleted entry */\n>> @@ -962,7 +963,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n>> \t}\n>>\n>> copy_elem:\n>> -\tj = n->pos++;\n>> +\tj = npos = n->pos + 1;\n>\n> Hmm. Should that be:\n> +\tj = npos;\n> +\tnpos = n->pos + 1;\n>\n> As-is j is advanced by 1 compared to old code.\n>\n>> \tdata = ahash_data(n, j, set->dsize);\n>> copy_data:\n>> \tt->hregion[r].elements++;\n>> @@ -985,6 +986,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n>> \tif (SET_WITH_TIMEOUT(set))\n>> \t\tip_set_timeout_set(ext_timeout(data, set), ext->timeout);\n>> \tsmp_mb__before_atomic();\n>> +\tn->pos = npos;\n>> \tset_bit(j, n->used);\n>\n> I think this needs a followup-patch to switch this to smp_store_release\n> and readers to smp_load_acquire helpers.\n\nYour comments are sharp, thanks! I'd better resend the batch in a new \nversion.\n\nBest regards,\nJozsef\n\n> Here is a diff for this (generated by LLM and only compile tested).\n> I think this can be a separate patch to not make this change too big and\n> to not mix different fixes.\n>\n> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h\n> index 0da02a8dfbae..2ca674e51699 100644\n> --- a/net/netfilter/ipset/ip_set_hash_gen.h\n> +++ b/net/netfilter/ipset/ip_set_hash_gen.h\n> @@ -682,10 +682,13 @@ mtype_resize(struct ip_set *set, bool retried)\n> \t\trcu_read_lock_bh();\n> \t\tfor (i = ahash_bucket_start(r, orig->htable_bits);\n> \t\t i < ahash_bucket_end(r, orig->htable_bits); i++) {\n> +\t\t\tu8 pos;\n> +\n> \t\t\tn = __ipset_dereference(hbucket(orig, i));\n> \t\t\tif (!n)\n> \t\t\t\tcontinue;\n> -\t\t\tfor (j = 0; j < n->pos; j++) {\n> +\t\t\tpos = smp_load_acquire(&n->pos);\n> +\t\t\tfor (j = 0; j < pos; j++) {\n> \t\t\t\tif (!test_bit(j, n->used))\n> \t\t\t\t\tcontinue;\n> \t\t\t\tdata = ahash_data(n, j, dsize);\n> @@ -817,10 +820,13 @@ mtype_ext_size(struct ip_set *set, u32 *elements, size_t *ext_size)\n> \tfor (r = 0; r < ahash_numof_locks(t->htable_bits); r++) {\n> \t\tfor (i = ahash_bucket_start(r, t->htable_bits);\n> \t\t i < ahash_bucket_end(r, t->htable_bits); i++) {\n> +\t\t\tu8 pos;\n> +\n> \t\t\tn = rcu_dereference_bh(hbucket(t, i));\n> \t\t\tif (!n)\n> \t\t\t\tcontinue;\n> -\t\t\tfor (j = 0; j < n->pos; j++) {\n> +\t\t\tpos = smp_load_acquire(&n->pos);\n> +\t\t\tfor (j = 0; j < pos; j++) {\n> \t\t\t\tif (!test_bit(j, n->used))\n> \t\t\t\t\tcontinue;\n> \t\t\t\tdata = ahash_data(n, j, set->dsize);\n> @@ -963,7 +969,8 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n> \t}\n>\n> copy_elem:\n> -\tj = npos = n->pos + 1;\n> +\tj = npos;\n> +\tnpos = n->pos + 1;\n> \tdata = ahash_data(n, j, set->dsize);\n> copy_data:\n> \tt->hregion[r].elements++;\n> @@ -985,8 +992,8 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n> \t/* Must come last for the case when timed out entry is reused */\n> \tif (SET_WITH_TIMEOUT(set))\n> \t\tip_set_timeout_set(ext_timeout(data, set), ext->timeout);\n> -\tsmp_mb__before_atomic();\n> -\tn->pos = npos;\n> +\t/* Ensure all data writes are visible before updating position */\n> +\tsmp_store_release(&n->pos, npos);\n> \tset_bit(j, n->used);\n> \tif (old != ERR_PTR(-ENOENT)) {\n> \t\trcu_assign_pointer(hbucket(t, key), n);\n> @@ -1172,6 +1179,7 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,\n> \tint ret, i, j = 0;\n> #endif\n> \tu32 key, multi = 0;\n> +\tu8 pos;\n>\n> \tpr_debug(\"test by nets\\n\");\n> \tfor (; j < NLEN && h->nets[j].cidr[0] && !multi; j++) {\n> @@ -1189,7 +1197,8 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,\n> \t\tn = rcu_dereference_bh(hbucket(t, key));\n> \t\tif (!n)\n> \t\t\tcontinue;\n> -\t\tfor (i = 0; i < n->pos; i++) {\n> +\t\tpos = smp_load_acquire(&n->pos);\n> +\t\tfor (i = 0; i < pos; i++) {\n> \t\t\tif (!test_bit(i, n->used))\n> \t\t\t\tcontinue;\n> \t\t\tdata = ahash_data(n, i, set->dsize);\n> @@ -1223,6 +1232,7 @@ mtype_test(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n> \tstruct mtype_elem *data;\n> \tint i, ret = 0;\n> \tu32 key, multi = 0;\n> +\tu8 pos;\n>\n> \trcu_read_lock_bh();\n> \tt = rcu_dereference_bh(h->table);\n> @@ -1245,7 +1255,8 @@ mtype_test(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n> \t\tret = 0;\n> \t\tgoto out;\n> \t}\n> -\tfor (i = 0; i < n->pos; i++) {\n> +\tpos = smp_load_acquire(&n->pos);\n> +\tfor (i = 0; i < pos; i++) {\n> \t\tif (!test_bit(i, n->used))\n> \t\t\tcontinue;\n> \t\tdata = ahash_data(n, i, set->dsize);\n> @@ -1373,6 +1384,8 @@ mtype_list(const struct ip_set *set,\n> \trcu_read_lock();\n> \tfor (; cb->args[IPSET_CB_ARG0] < jhash_size(t->htable_bits);\n> \t cb->args[IPSET_CB_ARG0]++) {\n> +\t\tu8 pos;\n> +\n> \t\tcond_resched_rcu();\n> \t\tincomplete = skb_tail_pointer(skb);\n> \t\tn = rcu_dereference(hbucket(t, cb->args[IPSET_CB_ARG0]));\n> @@ -1380,7 +1393,9 @@ mtype_list(const struct ip_set *set,\n> \t\t\t cb->args[IPSET_CB_ARG0], t, n);\n> \t\tif (!n)\n> \t\t\tcontinue;\n> -\t\tfor (i = 0; i < n->pos; i++) {\n> +\t\t/* Acquire ordering with smp_store_release in mtype_add */\n> +\t\tpos = smp_load_acquire(&n->pos);\n> +\t\tfor (i = 0; i < pos; i++) {\n> \t\t\tif (!test_bit(i, n->used))\n> \t\t\t\tcontinue;\n> \t\t\te = ahash_data(n, i, set->dsize);\n>\n>\n>","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","netfilter-devel@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (1024-bit key;\n unprotected) header.d=blackhole.kfki.hu header.i=@blackhole.kfki.hu\n header.a=rsa-sha256 header.s=20151130 header.b=o9gWWUjy;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=netfilter-devel+bounces-11911-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (1024-bit key) header.d=blackhole.kfki.hu\n header.i=@blackhole.kfki.hu header.b=\"o9gWWUjy\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=148.6.0.50","smtp.subspace.kernel.org;\n dmarc=pass (p=quarantine dis=none) header.from=blackhole.kfki.hu","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=blackhole.kfki.hu"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fwcv15yR7z1yDF\n\tfor ; Wed, 15 Apr 2026 20:33:49 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 247C5301BC2E\n\tfor ; Wed, 15 Apr 2026 10:33:44 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id B29BF33F5B5;\n\tWed, 15 Apr 2026 10:33:43 +0000 (UTC)","from smtp-out.kfki.hu (smtp-out.kfki.hu [148.6.0.50])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 85ADA328B5E\n\tfor ; Wed, 15 Apr 2026 10:33:40 +0000 (UTC)","from localhost (localhost [127.0.0.1])\n\tby smtp1.kfki.hu (Postfix) with ESMTP id 4fwctp51CpzGFDMM;\n\tWed, 15 Apr 2026 12:33:38 +0200 (CEST)","from smtp1.kfki.hu ([127.0.0.1])\n by localhost (smtp1.kfki.hu [127.0.0.1]) (amavis, port 10026) with ESMTP\n id RToBENkFv7JE; Wed, 15 Apr 2026 12:33:36 +0200 (CEST)","from blackhole.kfki.hu (blackhole.szhk.kfki.hu\n [IPv6:2001:738:5001:1::240:2])\n\tby smtp1.kfki.hu (Postfix) with ESMTP id 4fwctm5XqBzGFDM6;\n\tWed, 15 Apr 2026 12:33:36 +0200 (CEST)","by blackhole.kfki.hu (Postfix, from userid 1000)\n\tid AEEC334316A; Wed, 15 Apr 2026 12:33:36 +0200 (CEST)","from localhost (localhost [127.0.0.1])\n\tby blackhole.kfki.hu (Postfix) with ESMTP id AD1A5340D75;\n\tWed, 15 Apr 2026 12:33:36 +0200 (CEST)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776249223; cv=none;\n b=oXFY/L6PSaQXtHb85QOqDxiiMQL0CIddGBwLQNlrkC2Ya8LiP3BeHj6N2Lr6iwAo5cHtuvn0/0sdNnnOi7mUcpStZtm2IUpzf6+vV7LodhUNSjgQ4uFy6iFmXo1RCreOtnLKFvJ3TOu+StoDlQUXRX7fIF/OaehjpmBVk4WKFj0=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776249223; c=relaxed/simple;\n\tbh=GEztm2x3YJlz7eFkN2ag2hM5gd04XoP7eVga1xtupJU=;\n\th=Date:From:To:cc:Subject:In-Reply-To:Message-ID:References:\n\t MIME-Version:Content-Type;\n b=DS1bKOg3pm+3GHId1scy9hR4UURk/sPJFOjnWE+IdPBnXA6rkkD1m+Al0kG/MZ2Zxpp7Svde6QiO1ZMmU16Weaw+6Wj/XglhUIjnJhL1OLM42YyLI27oybIyOackHtat/pV313cuQYpyzB4E+owu9Fps5gunID1tgvcIr1Cq16k=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=quarantine dis=none) header.from=blackhole.kfki.hu;\n spf=pass smtp.mailfrom=blackhole.kfki.hu;\n dkim=pass (1024-bit key) header.d=blackhole.kfki.hu\n header.i=@blackhole.kfki.hu header.b=o9gWWUjy;\n arc=none smtp.client-ip=148.6.0.50","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tblackhole.kfki.hu; h=mime-version:references:message-id\n\t:in-reply-to:from:from:date:date:received:received:received\n\t:received; s=20151130; t=1776249216; x=1778063617; bh=24m1BNrb0V\n\t10LOtLF8eYd7yYZKEWgrXt9M3fhqFO2aQ=; b=o9gWWUjyCNq4DPJ7hsFW1BV+JS\n\tZa4xTbrof0jysuQgXjvh7DcxfX45fxsXe0sjkWWQop4xna6ioXp5YDlc1SZ7Af17\n\tAT+9YbFvWc5ysGiJQmw34cUQI3f1/lyBg+c8sYpo5QelIhNMhcxkbaO05IOz/8zQ\n\tmvDuIS72599Z+APCY=","X-Virus-Scanned":"Debian amavis at smtp1.kfki.hu","Date":"Wed, 15 Apr 2026 12:33:36 +0200 (CEST)","From":"Jozsef Kadlecsik ","To":"Florian Westphal ","cc":"Jozsef Kadlecsik , netfilter-devel@vger.kernel.org,\n Pablo Neira Ayuso ","Subject":"Re: [PATCH 2/2] netfilter: ipset: Fix data race between add and dump\n in all hash types","In-Reply-To":"","Message-ID":"<93dd963e-7577-83a5-244f-f19f49ddcd78@blackhole.kfki.hu>","References":"<20260415082039.4133308-1-kadlec@netfilter.org>\n <20260415082039.4133308-3-kadlec@netfilter.org> ","Precedence":"bulk","X-Mailing-List":"netfilter-devel@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII; format=flowed"}},{"id":3679404,"web_url":"http://patchwork.ozlabs.org/comment/3679404/","msgid":"<4e01c555-2f4c-81b7-e6c4-d1f7b3e2e99f@blackhole.kfki.hu>","list_archive_url":null,"date":"2026-04-20T13:11:20","subject":"Re: [PATCH 2/2] netfilter: ipset: Fix data race between add and dump\n in all hash types","submitter":{"id":1575,"url":"http://patchwork.ozlabs.org/api/people/1575/","name":"Jozsef Kadlecsik","email":"kadlec@blackhole.kfki.hu"},"content":"Hi Florian,\n\nOn Wed, 15 Apr 2026, Florian Westphal wrote:\n\n>> \tdata = ahash_data(n, j, set->dsize);\n>> copy_data:\n>> \tt->hregion[r].elements++;\n>> @@ -985,6 +986,7 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n>> \tif (SET_WITH_TIMEOUT(set))\n>> \t\tip_set_timeout_set(ext_timeout(data, set), ext->timeout);\n>> \tsmp_mb__before_atomic();\n>> +\tn->pos = npos;\n>> \tset_bit(j, n->used);\n>\n> I think this needs a followup-patch to switch this to smp_store_release\n> and readers to smp_load_acquire helpers.\n\nI'm pondering over the helpers and unsure. The hash bucket structure is as \nfollows:\n\n/* A hash bucket */\nstruct hbucket {\n struct rcu_head rcu; /* for call_rcu */\n /* Which positions are used in the array */\n DECLARE_BITMAP(used, AHASH_MAX_TUNED);\n u8 size; /* size of the array */\n u8 pos; /* position of the first free entry */\n unsigned char value[] /* the array of the values */\n __aligned(__alignof__(u64));\n};\n\nThe elements in \"value\" are filled up successively, \"pos\" is there to \nlimit the searching range for existing elements (which might be timed out \nas well). So until the \"size\" is unchanged (no growing/shrinking), the \nworst things which can happen if it's not \"correct\":\n\n- new element added but dump/test don't \"find\" it\n- element deleted but dump/test find it\n\nThe critical part is when \"size\" changes. But \"size\" never updated \ndirectly: a completely new bucket is created when growing/shrinking and \nthe new bucket is assigned via RCU mechanism.\n\nSo why do you thing the helpers are required to read/update the \"pos\" \nelement of the hash bucket? I might not see the wood for the trees.\n\nBest regards,\nJozsef\n\n> Here is a diff for this (generated by LLM and only compile tested).\n> I think this can be a separate patch to not make this change too big and\n> to not mix different fixes.\n>\n> diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h\n> index 0da02a8dfbae..2ca674e51699 100644\n> --- a/net/netfilter/ipset/ip_set_hash_gen.h\n> +++ b/net/netfilter/ipset/ip_set_hash_gen.h\n> @@ -682,10 +682,13 @@ mtype_resize(struct ip_set *set, bool retried)\n> \t\trcu_read_lock_bh();\n> \t\tfor (i = ahash_bucket_start(r, orig->htable_bits);\n> \t\t i < ahash_bucket_end(r, orig->htable_bits); i++) {\n> +\t\t\tu8 pos;\n> +\n> \t\t\tn = __ipset_dereference(hbucket(orig, i));\n> \t\t\tif (!n)\n> \t\t\t\tcontinue;\n> -\t\t\tfor (j = 0; j < n->pos; j++) {\n> +\t\t\tpos = smp_load_acquire(&n->pos);\n> +\t\t\tfor (j = 0; j < pos; j++) {\n> \t\t\t\tif (!test_bit(j, n->used))\n> \t\t\t\t\tcontinue;\n> \t\t\t\tdata = ahash_data(n, j, dsize);\n> @@ -817,10 +820,13 @@ mtype_ext_size(struct ip_set *set, u32 *elements, size_t *ext_size)\n> \tfor (r = 0; r < ahash_numof_locks(t->htable_bits); r++) {\n> \t\tfor (i = ahash_bucket_start(r, t->htable_bits);\n> \t\t i < ahash_bucket_end(r, t->htable_bits); i++) {\n> +\t\t\tu8 pos;\n> +\n> \t\t\tn = rcu_dereference_bh(hbucket(t, i));\n> \t\t\tif (!n)\n> \t\t\t\tcontinue;\n> -\t\t\tfor (j = 0; j < n->pos; j++) {\n> +\t\t\tpos = smp_load_acquire(&n->pos);\n> +\t\t\tfor (j = 0; j < pos; j++) {\n> \t\t\t\tif (!test_bit(j, n->used))\n> \t\t\t\t\tcontinue;\n> \t\t\t\tdata = ahash_data(n, j, set->dsize);\n> @@ -963,7 +969,8 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n> \t}\n>\n> copy_elem:\n> -\tj = npos = n->pos + 1;\n> +\tj = npos;\n> +\tnpos = n->pos + 1;\n> \tdata = ahash_data(n, j, set->dsize);\n> copy_data:\n> \tt->hregion[r].elements++;\n> @@ -985,8 +992,8 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n> \t/* Must come last for the case when timed out entry is reused */\n> \tif (SET_WITH_TIMEOUT(set))\n> \t\tip_set_timeout_set(ext_timeout(data, set), ext->timeout);\n> -\tsmp_mb__before_atomic();\n> -\tn->pos = npos;\n> +\t/* Ensure all data writes are visible before updating position */\n> +\tsmp_store_release(&n->pos, npos);\n> \tset_bit(j, n->used);\n> \tif (old != ERR_PTR(-ENOENT)) {\n> \t\trcu_assign_pointer(hbucket(t, key), n);\n> @@ -1172,6 +1179,7 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,\n> \tint ret, i, j = 0;\n> #endif\n> \tu32 key, multi = 0;\n> +\tu8 pos;\n>\n> \tpr_debug(\"test by nets\\n\");\n> \tfor (; j < NLEN && h->nets[j].cidr[0] && !multi; j++) {\n> @@ -1189,7 +1197,8 @@ mtype_test_cidrs(struct ip_set *set, struct mtype_elem *d,\n> \t\tn = rcu_dereference_bh(hbucket(t, key));\n> \t\tif (!n)\n> \t\t\tcontinue;\n> -\t\tfor (i = 0; i < n->pos; i++) {\n> +\t\tpos = smp_load_acquire(&n->pos);\n> +\t\tfor (i = 0; i < pos; i++) {\n> \t\t\tif (!test_bit(i, n->used))\n> \t\t\t\tcontinue;\n> \t\t\tdata = ahash_data(n, i, set->dsize);\n> @@ -1223,6 +1232,7 @@ mtype_test(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n> \tstruct mtype_elem *data;\n> \tint i, ret = 0;\n> \tu32 key, multi = 0;\n> +\tu8 pos;\n>\n> \trcu_read_lock_bh();\n> \tt = rcu_dereference_bh(h->table);\n> @@ -1245,7 +1255,8 @@ mtype_test(struct ip_set *set, void *value, const struct ip_set_ext *ext,\n> \t\tret = 0;\n> \t\tgoto out;\n> \t}\n> -\tfor (i = 0; i < n->pos; i++) {\n> +\tpos = smp_load_acquire(&n->pos);\n> +\tfor (i = 0; i < pos; i++) {\n> \t\tif (!test_bit(i, n->used))\n> \t\t\tcontinue;\n> \t\tdata = ahash_data(n, i, set->dsize);\n> @@ -1373,6 +1384,8 @@ mtype_list(const struct ip_set *set,\n> \trcu_read_lock();\n> \tfor (; cb->args[IPSET_CB_ARG0] < jhash_size(t->htable_bits);\n> \t cb->args[IPSET_CB_ARG0]++) {\n> +\t\tu8 pos;\n> +\n> \t\tcond_resched_rcu();\n> \t\tincomplete = skb_tail_pointer(skb);\n> \t\tn = rcu_dereference(hbucket(t, cb->args[IPSET_CB_ARG0]));\n> @@ -1380,7 +1393,9 @@ mtype_list(const struct ip_set *set,\n> \t\t\t cb->args[IPSET_CB_ARG0], t, n);\n> \t\tif (!n)\n> \t\t\tcontinue;\n> -\t\tfor (i = 0; i < n->pos; i++) {\n> +\t\t/* Acquire ordering with smp_store_release in mtype_add */\n> +\t\tpos = smp_load_acquire(&n->pos);\n> +\t\tfor (i = 0; i < pos; i++) {\n> \t\t\tif (!test_bit(i, n->used))\n> \t\t\t\tcontinue;\n> \t\t\te = ahash_data(n, i, set->dsize);\n>\n>\n>","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","netfilter-devel@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (1024-bit key;\n unprotected) header.d=blackhole.kfki.hu header.i=@blackhole.kfki.hu\n header.a=rsa-sha256 header.s=20151130 header.b=oM/3mySC;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.234.253.10; helo=sea.lore.kernel.org;\n envelope-from=netfilter-devel+bounces-12052-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (1024-bit key) header.d=blackhole.kfki.hu\n header.i=@blackhole.kfki.hu header.b=\"oM/3mySC\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=148.6.0.51","smtp.subspace.kernel.org;\n dmarc=pass (p=quarantine dis=none) header.from=blackhole.kfki.hu","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=blackhole.kfki.hu"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org [172.234.253.10])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fzmHm6FH6z1yCv\n\tfor ; Mon, 20 Apr 2026 23:17:40 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 4963530C32E4\n\tfor ; Mon, 20 Apr 2026 13:11:34 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 580653A0E84;\n\tMon, 20 Apr 2026 13:11:33 +0000 (UTC)","from smtp-out.kfki.hu (smtp-out.kfki.hu [148.6.0.51])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id C8B81399019\n\tfor ; Mon, 20 Apr 2026 13:11:29 +0000 (UTC)","from localhost (localhost [127.0.0.1])\n\tby smtp2.kfki.hu (Postfix) with ESMTP id 4fzm8V25yYz7s85C;\n\tMon, 20 Apr 2026 15:11:22 +0200 (CEST)","from smtp2.kfki.hu ([127.0.0.1])\n by localhost (smtp2.kfki.hu [127.0.0.1]) (amavis, port 10026) with ESMTP\n id AaIniT8gAok0; Mon, 20 Apr 2026 15:11:20 +0200 (CEST)","from blackhole.kfki.hu (blackhole.szhk.kfki.hu [148.6.240.2])\n\tby smtp2.kfki.hu (Postfix) with ESMTP id 4fzm8S17fBz7s856;\n\tMon, 20 Apr 2026 15:11:20 +0200 (CEST)","by blackhole.kfki.hu (Postfix, from userid 1000)\n\tid 1B75334316A; Mon, 20 Apr 2026 15:11:20 +0200 (CEST)","from localhost (localhost [127.0.0.1])\n\tby blackhole.kfki.hu (Postfix) with ESMTP id 1976F340D75;\n\tMon, 20 Apr 2026 15:11:20 +0200 (CEST)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776690692; cv=none;\n b=pf7UUjH9MsjXunV9dNeS5FAQCeCaWqUkMgnnhF/lGo60C6YSpQ5ksn0W66h6maMRfQQsmUWjsAevnnzunrXoIDdWveMb0DunjHOU1MzqzvawiMCZCj/38zVU5wImS8xOuDkqYrDaVOKsLKOAeJmxwv7IpEx7R1CAkXmxOrj6KF8=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776690692; c=relaxed/simple;\n\tbh=6X9w+a1doJSDKdQBjPF0L27atXhSk1iynvIRjrh6O7o=;\n\th=Date:From:To:cc:Subject:In-Reply-To:Message-ID:References:\n\t MIME-Version:Content-Type;\n b=EgBD9fC1NVFD6JKygSddq/DYbPuSO6mGbH0uw+HDf6BGZUATDTB4u951/tJkMcf/8D0bZAVhiC2yG38gcQ2gjV84eA63PVTSP6WLJu280F2SidmvjW7qmB3XnrdnISXpWCV9/yRE7O45DcFmRyXOmkXbxMPIkSzi2NCv5vlX6A4=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=quarantine dis=none) header.from=blackhole.kfki.hu;\n spf=pass smtp.mailfrom=blackhole.kfki.hu;\n dkim=pass (1024-bit key) header.d=blackhole.kfki.hu\n header.i=@blackhole.kfki.hu header.b=oM/3mySC;\n arc=none smtp.client-ip=148.6.0.51","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tblackhole.kfki.hu; h=mime-version:references:message-id\n\t:in-reply-to:from:from:date:date:received:received:received\n\t:received; s=20151130; t=1776690680; x=1778505081; bh=aVfqHj2Jyw\n\tBWpt+gfxLGgQSvAZL4GsZ86QxpWgIr+yo=; b=oM/3mySCUgVhjLdBkL5S9MCcHO\n\tl5g7ZLdEzm2sbwtsyk7VU/JIRe+cRMThHfAi9EqlIYFO9sxXPo49JGeDAfojW/F0\n\t8SRDuyqs62Q5GjmCu52a/fohkcwnzb+xMRpsykPP8y78CkpjR7RkySfB6vsxq7I0\n\t62mVKxzNb1F08/3W8=","X-Virus-Scanned":"Debian amavis at smtp2.kfki.hu","Date":"Mon, 20 Apr 2026 15:11:20 +0200 (CEST)","From":"Jozsef Kadlecsik ","To":"Florian Westphal ","cc":"Jozsef Kadlecsik , netfilter-devel@vger.kernel.org,\n Pablo Neira Ayuso ","Subject":"Re: [PATCH 2/2] netfilter: ipset: Fix data race between add and dump\n in all hash types","In-Reply-To":"","Message-ID":"<4e01c555-2f4c-81b7-e6c4-d1f7b3e2e99f@blackhole.kfki.hu>","References":"<20260415082039.4133308-1-kadlec@netfilter.org>\n <20260415082039.4133308-3-kadlec@netfilter.org> ","Precedence":"bulk","X-Mailing-List":"netfilter-devel@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII; format=flowed"}},{"id":3679436,"web_url":"http://patchwork.ozlabs.org/comment/3679436/","msgid":"","list_archive_url":null,"date":"2026-04-20T14:03:26","subject":"Re: [PATCH 2/2] netfilter: ipset: Fix data race between add and dump\n in all hash types","submitter":{"id":1025,"url":"http://patchwork.ozlabs.org/api/people/1025/","name":"Florian Westphal","email":"fw@strlen.de"},"content":"Jozsef Kadlecsik wrote:\n> limit the searching range for existing elements (which might be timed out \n> as well). So until the \"size\" is unchanged (no growing/shrinking), the \n> worst things which can happen if it's not \"correct\":\n> \n> - new element added but dump/test don't \"find\" it\n> - element deleted but dump/test find it\n> \n> The critical part is when \"size\" changes. But \"size\" never updated \n> directly: a completely new bucket is created when growing/shrinking and \n> the new bucket is assigned via RCU mechanism.\n> \n> So why do you thing the helpers are required to read/update the \"pos\" \n> element of the hash bucket? I might not see the wood for the trees.\n\nAs-is it is confusing. Pos is updated concurrently to dumpers\nwith no ordering and no annotations (READ_ONCE, WRITE_ONCE).\n\nMaybe they are not required, but then there should be WRITE_ONCE()\nand READ_ONCE and a comment explaining that this is deliberately racy.\n\nBasically what you wrote above. It would help to understand what\nguarantees there are and that this is good enough.\n\nAs-is KCSAN will surely splat without data race annotations.","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","netfilter-devel@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=netfilter-devel+bounces-12065-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=91.216.245.30","smtp.subspace.kernel.org;\n dmarc=none (p=none dis=none) header.from=strlen.de","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=strlen.de"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fzq2W1ctjz1yD4\n\tfor ; Tue, 21 Apr 2026 01:21:23 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 1700733703AA\n\tfor ; Mon, 20 Apr 2026 14:53:57 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 407773603EE;\n\tMon, 20 Apr 2026 14:03:38 +0000 (UTC)","from Chamillionaire.breakpoint.cc (Chamillionaire.breakpoint.cc\n [91.216.245.30])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 2751C35B64B\n\tfor ; Mon, 20 Apr 2026 14:03:35 +0000 (UTC)","by Chamillionaire.breakpoint.cc (Postfix, from userid 1003)\n\tid 5D55460490; Mon, 20 Apr 2026 16:03:27 +0200 (CEST)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776693817; cv=none;\n b=m7i7uQneOdUqfyr/1BislmydEM7f1865jr68apxoeRsyNj5FP1SPlhbWCybvGecKWdvnFnnZFlrzQTvgO1HLqgTkXIpdoccab4FsalIciGU3M85xP0iLGd1NOCzKW0gpFbSBBXD3VbjF03LbYv5vEAF/gRsJukr2/QWo3TKg0mQ=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776693817; c=relaxed/simple;\n\tbh=hMzUSV3qYUm7kcpMbXCteh3du5pUrpurDeRDnvmQd6o=;\n\th=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:\n\t Content-Type:Content-Disposition:In-Reply-To;\n b=M7qZmO0HcOWXswxTnsY2OU4TiBx2IPQKIHzpdZ+aqjiibIZbkkv3JMY7M6CDL1jn3uRtyVNhCYjQJ9Fkt4uBn/OfBviotsL5OOcasrv40induTcOGdNHUxO/QMqv0I2MaEq7VlMTxq9ZYW+JlZC73rWGKkdlBx9N3ak3RYAuuzM=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=none (p=none dis=none) header.from=strlen.de;\n spf=pass smtp.mailfrom=strlen.de; arc=none smtp.client-ip=91.216.245.30","Date":"Mon, 20 Apr 2026 16:03:26 +0200","From":"Florian Westphal ","To":"Jozsef Kadlecsik ","Cc":"Jozsef Kadlecsik ,\n\tnetfilter-devel@vger.kernel.org,\n\tPablo Neira Ayuso ","Subject":"Re: [PATCH 2/2] netfilter: ipset: Fix data race between add and dump\n in all hash types","Message-ID":"","References":"<20260415082039.4133308-1-kadlec@netfilter.org>\n <20260415082039.4133308-3-kadlec@netfilter.org>\n \n <4e01c555-2f4c-81b7-e6c4-d1f7b3e2e99f@blackhole.kfki.hu>","Precedence":"bulk","X-Mailing-List":"netfilter-devel@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<4e01c555-2f4c-81b7-e6c4-d1f7b3e2e99f@blackhole.kfki.hu>"}}]