diff mbox series

netfilter: ipset: wait for xt_recseq on all cpus

Message ID 20231005115022.12902-1-xiaolinkui@126.com (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series netfilter: ipset: wait for xt_recseq on all cpus | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1344 this patch: 1344
netdev/cc_maintainers success CCed 12 of 12 maintainers
netdev/build_clang success Errors and warnings before: 1363 this patch: 1363
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1368 this patch: 1368
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 38 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

xiaolinkui Oct. 5, 2023, 11:50 a.m. UTC
From: Linkui Xiao <xiaolinkui@kylinos.cn>

Before destroying the ipset, take a check on sequence to ensure that the
ip_set_test operation of this ipset has been completed.

The code of set_match_v4 is protected by addend=xt_write_recseq_begin() and
xt_write_recseq_end(addend). So we can ensure that the test operation is
completed by reading seqcount.

Otherwise, there will be a low probability of use-after-free problems
occurring:

 PC: ffff0000033c0168  [hash_net4_kadt+56]
 LR: ffff000002b811bc  [ip_set_test+188]
 SP: ffff8003fff3f8d0  PSTATE: 60400005
X29: ffff8003fff3f8d0  X28: ffff8003ab915c4e  X27: ffff8003b0c7a000
X26: ffff8003b9780040  X25: ffff000000c70600  X24: ffff8003ac2c0200
X23: ffff000002f70fcc  X22: 0000000000000002  X21: ffff8003ac2c0200
X20: ffff8003be8e2800  X19: ffff8003fff3f9c8  X18: 0000000000000000
X17: 0000000000000000  X16: 0000000000000000  X15: 0000000000000000
X14: 970000002d494600  X13: 0000000000000000  X12: c5d405f139e6e418
X11: ffff000000c70600  X10: ffff8003b0c7a000   X9: 0000000000000001
 X8: 0000000000000000   X7: 000000000000005f   X6: 0000000000000000
 X5: ffff0000033c0130   X4: ffff8003fff3f9c8   X3: 0000000000000002
 X2: ffff0000033d01d8   X1: 00000000ffffffff   X0: 0000000000000000
[ffff8003fff3f8d0] hash_net4_kadt at ffff0000033c0164 [ip_set_hash_net]
[ffff8003fff3f940] ip_set_test at ffff000002b811b8 [ip_set]
[ffff8003fff3f990] set_match_v4 at ffff000002f70fc8 [xt_set]
[ffff8003fff3fa20] ipt_do_table at ffff000000c504e0 [ip_tables]
[ffff8003fff3fb60] iptable_filter_hook at ffff00000266006c [iptable_filter]
[ffff8003fff3fb80] nf_hook_slow at ffff000008ac7a84
[ffff8003fff3fbc0] ip_local_deliver at ffff000008ad5d88
[ffff8003fff3fc10] ip_rcv_finish at ffff000008ad59b4
[ffff8003fff3fc40] ip_rcv at ffff000008ad5dec
[ffff8003fff3fca0] __netif_receive_skb_one_core at ffff000008a6c344
[ffff8003fff3fce0] __netif_receive_skb at ffff000008a6c3ac
[ffff8003fff3fd00] netif_receive_skb_internal at ffff000008a6c440
[ffff8003fff3fd30] napi_gro_receive at ffff000008a6d3ec
[ffff8003fff3fd60] receive_buf at ffff000001c934d8 [virtio_net]
[ffff8003fff3fe20] virtnet_poll at ffff000001c953e8 [virtio_net]
[ffff8003fff3fec0] net_rx_action at ffff000008a6c9ec
[ffff8003fff3ff60] __softirqentry_text_start at ffff0000080819f0
[ffff8003fff3fff0] irq_exit at ffff0000080f1228
[ffff8003fff40010] __handle_domain_irq at ffff000008162a10

Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
 net/netfilter/ipset/ip_set_core.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Florian Westphal Oct. 8, 2023, 12:19 p.m. UTC | #1
xiaolinkui <xiaolinkui@126.com> wrote:
> crash> struct seqcount_t ffff8003fff3bf88
> struct seqcount_t {
>   sequence = 804411271
> }
> 
> The seqcount of CPU7 is odd, xt_replace_table should have waited, but it
> didn't.

Likely missing backport of
175e476b8cdf ("netfilter: x_tables: Use correct memory barriers.")?
kernel test robot Oct. 16, 2023, 8:51 a.m. UTC | #2
Hi xiaolinkui,

kernel test robot noticed the following build errors:

[auto build test ERROR on netfilter-nf/main]
[also build test ERROR on nf-next/master horms-ipvs/master linus/master v6.6-rc6 next-20231016]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/xiaolinkui/netfilter-ipset-wait-for-xt_recseq-on-all-cpus/20231005-234042
base:   git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git main
patch link:    https://lore.kernel.org/r/20231005115022.12902-1-xiaolinkui%40126.com
patch subject: [PATCH] netfilter: ipset: wait for xt_recseq on all cpus
config: x86_64-buildonly-randconfig-006-20231016 (https://download.01.org/0day-ci/archive/20231016/202310161625.GDDBP8SZ-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231016/202310161625.GDDBP8SZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310161625.GDDBP8SZ-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: vmlinux.o: in function `wait_xt_recseq':
>> ip_set_core.c:(.text+0x1c561ac): undefined reference to `xt_recseq'
kernel test robot Oct. 16, 2023, 9:14 a.m. UTC | #3
Hi xiaolinkui,

kernel test robot noticed the following build errors:

[auto build test ERROR on netfilter-nf/main]
[also build test ERROR on nf-next/master horms-ipvs/master linus/master v6.6-rc6 next-20231016]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/xiaolinkui/netfilter-ipset-wait-for-xt_recseq-on-all-cpus/20231005-234042
base:   git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git main
patch link:    https://lore.kernel.org/r/20231005115022.12902-1-xiaolinkui%40126.com
patch subject: [PATCH] netfilter: ipset: wait for xt_recseq on all cpus
config: i386-randconfig-016-20231016 (https://download.01.org/0day-ci/archive/20231016/202310161728.mW3lt1Jl-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231016/202310161728.mW3lt1Jl-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310161728.mW3lt1Jl-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: net/netfilter/ipset/ip_set_core.o: in function `wait_xt_recseq':
>> net/netfilter/ipset/ip_set_core.c:1194: undefined reference to `xt_recseq'


vim +1194 net/netfilter/ipset/ip_set_core.c

  1187	
  1188	static void wait_xt_recseq(void)
  1189	{
  1190		unsigned int cpu;
  1191	
  1192		/* wait for even xt_recseq on all cpus */
  1193		for_each_possible_cpu(cpu) {
> 1194			seqcount_t *s = &per_cpu(xt_recseq, cpu);
  1195			u32 seq = raw_read_seqcount(s);
  1196	
  1197			if (seq & 1) {
  1198				do {
  1199					cond_resched();
  1200					cpu_relax();
  1201				} while (seq == raw_read_seqcount(s));
  1202			}
  1203		}
  1204	}
  1205
Jozsef Kadlecsik Oct. 16, 2023, 12:56 p.m. UTC | #4
Hello,

Besides the broken patch, the description simply cannot be true:

"Before destroying the ipset, take a check on sequence to ensure that the 
ip_set_test operation of this ipset has been completed."

Set can only be destroyed when there is no iptables rule (match/target) 
which refers to it. If this condition is not true, then the real reason 
must be fixed.

How can one reproduce the issue?

Best regards,
Jozsef

On Mon, 16 Oct 2023, kernel test robot wrote:

> Hi xiaolinkui,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on netfilter-nf/main]
> [also build test ERROR on nf-next/master horms-ipvs/master linus/master v6.6-rc6 next-20231016]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/xiaolinkui/netfilter-ipset-wait-for-xt_recseq-on-all-cpus/20231005-234042
> base:   git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git main
> patch link:    https://lore.kernel.org/r/20231005115022.12902-1-xiaolinkui%40126.com
> patch subject: [PATCH] netfilter: ipset: wait for xt_recseq on all cpus
> config: x86_64-buildonly-randconfig-006-20231016 (https://download.01.org/0day-ci/archive/20231016/202310161625.GDDBP8SZ-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231016/202310161625.GDDBP8SZ-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202310161625.GDDBP8SZ-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>    ld: vmlinux.o: in function `wait_xt_recseq':
> >> ip_set_core.c:(.text+0x1c561ac): undefined reference to `xt_recseq'
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>
diff mbox series

Patch

diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 46f4f47e29e4..53561176162f 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1187,6 +1187,24 @@  ip_set_destroy_set(struct ip_set *set)
 	kfree(set);
 }
 
+static void wait_xt_recseq(void)
+{
+	unsigned int cpu;
+
+	/* wait for even xt_recseq on all cpus */
+	for_each_possible_cpu(cpu) {
+		seqcount_t *s = &per_cpu(xt_recseq, cpu);
+		u32 seq = raw_read_seqcount(s);
+
+		if (seq & 1) {
+			do {
+				cond_resched();
+				cpu_relax();
+			} while (seq == raw_read_seqcount(s));
+		}
+	}
+}
+
 static int ip_set_destroy(struct sk_buff *skb, const struct nfnl_info *info,
 			  const struct nlattr * const attr[])
 {
@@ -1225,6 +1243,7 @@  static int ip_set_destroy(struct sk_buff *skb, const struct nfnl_info *info,
 		for (i = 0; i < inst->ip_set_max; i++) {
 			s = ip_set(inst, i);
 			if (s) {
+				wait_xt_recseq();
 				ip_set(inst, i) = NULL;
 				ip_set_destroy_set(s);
 			}
@@ -1243,6 +1262,7 @@  static int ip_set_destroy(struct sk_buff *skb, const struct nfnl_info *info,
 			ret = -IPSET_ERR_BUSY;
 			goto out;
 		}
+		wait_xt_recseq();
 		ip_set(inst, i) = NULL;
 		read_unlock_bh(&ip_set_ref_lock);