linux-kernel - [PATCH 5.9 RT] net: openvswitch: Fix using smp_processor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20201009124759.592550-1-juri.lelli@redhat.com>
Date:   Fri,  9 Oct 2020 14:47:59 +0200
From:   Juri Lelli <juri.lelli@...hat.com>
To:     tglx@...utronix.de, bigeasy@...utronix.de
Cc:     linux-rt-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        bristot@...hat.com, williams@...hat.com, echaudro@...hat.com,
        atheurer@...hat.com, Juri Lelli <juri.lelli@...hat.com>
Subject: [PATCH 5.9 RT] net: openvswitch: Fix using smp_processor_id() in preemptible code

The following BUG has been reported (slightly edited):

 BUG: using smp_processor_id() in preemptible [00000000] code: handler106/3082
 caller is flow_lookup.isra.15+0x2c/0xf0 [openvswitch]
 CPU: 46 PID: 3082 Comm: handler106 Not tainted ... #1
 Hardware name: Dell Inc. PowerEdge R640/06DKY5, BIOS 2.5.4 01/13/2020
 Call Trace:
  dump_stack+0x5c/0x80
  check_preemption_disabled+0xc4/0xd0
  flow_lookup.isra.15+0x2c/0xf0 [openvswitch]
  ovs_flow_tbl_lookup+0x3b/0x60 [openvswitch]
  ovs_flow_cmd_new+0x2d8/0x430 [openvswitch]
  ? __switch_to_asm+0x35/0x70
  ? __switch_to_asm+0x41/0x70
  ? __switch_to_asm+0x35/0x70
  genl_family_rcv_msg+0x1d7/0x410
  ? migrate_enable+0x123/0x3a0
  genl_rcv_msg+0x47/0x8c
  ? __kmalloc_node_track_caller+0xff/0x2e0
  ? genl_family_rcv_msg+0x410/0x410
  netlink_rcv_skb+0x4c/0x120
  genl_rcv+0x24/0x40
  netlink_unicast+0x197/0x230
  netlink_sendmsg+0x204/0x3d0
  sock_sendmsg+0x4c/0x50
  ___sys_sendmsg+0x29f/0x300
  ? migrate_enable+0x123/0x3a0
  ? ep_send_events_proc+0x8a/0x1f0
  ? ep_scan_ready_list.constprop.23+0x237/0x260
  ? rt_spin_unlock+0x23/0x40
  ? ep_poll+0x1b3/0x390
  ? __fget+0x72/0xa0
  __sys_sendmsg+0x57/0xa0
  do_syscall_64+0x87/0x1a0
  entry_SYSCALL_64_after_hwframe+0x65/0xca
 RIP: 0033:0x7f1ed72ccb07
 Code: ...
 RSP: 002b:00007f1ecbd9ba80 EFLAGS: 00003293 ORIG_RAX: 000000000000002e
 RAX: ffffffffffffffda RBX: 000000000000007b RCX: 00007f1ed72ccb07
 RDX: 0000000000000000 RSI: 00007f1ecbd9bb10 RDI: 000000000000007b
 RBP: 00007f1ecbd9bb10 R08: 0000000000000000 R09: 00007f1ecbd9d390
 R10: 0000000019616156 R11: 0000000000003293 R12: 0000000000000000
 R13: 00007f1ecbd9d338 R14: 00007f1ecbd9bfb0 R15: 00007f1ecbd9bb10

This happens because openvswitch/flow_table::flow_lookup() accesses
per-cpu data while being preemptible (and migratable).

Fix it by adding get/put_cpu_light(), so that, even if preempted, the
task executing this code is not migrated (operation is also guarded by
ovs_mutex mutex).

Signed-off-by: Juri Lelli <juri.lelli@...hat.com>
---
 net/openvswitch/flow_table.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
index e2235849a57e..7df27ef7da09 100644
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c
@@ -732,11 +732,14 @@ static struct sw_flow *flow_lookup(struct flow_table *tbl,
 				   u32 *n_cache_hit,
 				   u32 *index)
 {
-	u64 *usage_counters = this_cpu_ptr(ma->masks_usage_cntr);
+	u64 *usage_counters;
 	struct sw_flow *flow;
 	struct sw_flow_mask *mask;
 	int i;
 
+	get_cpu_light();
+	usage_counters = this_cpu_ptr(ma->masks_usage_cntr);
+
 	if (likely(*index < ma->max)) {
 		mask = rcu_dereference_ovsl(ma->masks[*index]);
 		if (mask) {
@@ -746,6 +749,7 @@ static struct sw_flow *flow_lookup(struct flow_table *tbl,
 				usage_counters[*index]++;
 				u64_stats_update_end(&ma->syncp);
 				(*n_cache_hit)++;
+				put_cpu_light();
 				return flow;
 			}
 		}
@@ -766,10 +770,12 @@ static struct sw_flow *flow_lookup(struct flow_table *tbl,
 			u64_stats_update_begin(&ma->syncp);
 			usage_counters[*index]++;
 			u64_stats_update_end(&ma->syncp);
+			put_cpu_light();
 			return flow;
 		}
 	}
 
+	put_cpu_light();
 	return NULL;
 }
 
-- 
2.26.2