[ovs-dev] [PATCHv4] datapath: do not use vport type to determine presence of Geneve attributes

Ansis Atteka aatteka at nicira.com
Fri Aug 1 21:26:43 UTC 2014


This patch fixes following kernel crash that could happen, if geneve
vport was not added yet, but revalidator thread attempted to dump flows.
To reproduce:
1. switch tunnel type between geneve and gre in a loop; and
2. run ping.

BUG: unable to handle kernel NULL pointer dereference at 0000000000000048
IP: [<ffffffffa0385470>] ovs_nla_put_flow+0x3d0/0x7c0 [openvswitch]
PGD 3b32b067 PUD 3b2ef067 PMD 0
Oops: 0000 [#2] SMP
...
CPU: 0 PID: 6450 Comm: revalidator2 Tainted: GF     D    O
3.13.0-24-generic #46-Ubuntu
Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference
Platform, BIOS 6.00 07/02/2012
task: ffff88003b4aafe0 ti: ffff88003d314000 task.ti: ffff88003d314000
RIP: 0010:[<ffffffffa0385470>]  [<ffffffffa0385470>]
ovs_nla_put_flow+0x3d0/0x7c0 [openvswitch]
RSP: 0018:ffff88003d315a10  EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff88003a9a9960 RCX: 0000000000000000
RDX: 0000000000000002 RSI: ffffffffffffffc8 RDI: ffff88003babcb80
RBP: ffff88003d315a68 R08: 0000000000000000 R09: 0000000000000004
R10: ffff880039c23034 R11: 0000000000000008 R12: ffff88003a861600
R13: ffff88003a9a9960 R14: ffff88003babcb80 R15: qffff88003a861600
FS:  00007ff0f5d94700(0000) GS:ffff88003f600000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000048 CR3: 000000003b55b000 CR4: 00000000000007f0
Stack:
ffffffff81385093 0000000000000000 0000000000000000 0000000000000000
ffff880000000000 ffff88003d315a58 ffff880039c23014 ffff88003a9a97a0
ffff88003babcb80 ffff880039c23018 ffff88003a861600 ffff88003d315ad0
Call Trace:
[<ffffffff81385093>] ? __nla_reserve+0x43/0x50
[<ffffffffa037e683>] ovs_flow_cmd_fill_info+0x93/0x2b0 [openvswitch]
[<ffffffffa0387159>] ? ovs_flow_tbl_dump_next+0x49/0xc0 [openvswitch]
[<ffffffffa037e920>] ovs_flow_cmd_dump+0x80/0xd0 [openvswitch]
[<ffffffff81645004>] netlink_dump+0x84/0x240
[<ffffffff816458eb>] __netlink_dump_start+0x1ab/0x220
[<ffffffff816498d7>] genl_family_rcv_msg+0x337/0x370
[<ffffffffa037e8a0>] ? ovs_flow_cmd_fill_info+0x2b0/0x2b0 [openvswitch]
[<ffffffff811a2778>] ? __kmalloc_node_track_caller+0x58/0x1e0
[<ffffffff81649910>] ? genl_family_rcv_msg+0x370/0x370
[<ffffffff816499a1>] genl_rcv_msg+0x91/0xd0
[<ffffffff81647a29>] netlink_rcv_skb+0xa9/0xc0
[<ffffffff81647f28>] genl_rcv+0x28/0x40
[<ffffffff81647055>] netlink_unicast+0xd5/0x1b0
[<ffffffff8164742f>] netlink_sendmsg+0x2ff/0x740
[<ffffffff816024eb>] sock_sendmsg+0x8b/0xc0
[<ffffffff811bbaa1>] ? __sb_end_write+0x31/0x60
[<ffffffff811d42bf>] ? touch_atime+0x10f/0x140
[<ffffffff811c2471>] ? pipe_read+0x371/0x400
[<ffffffff81602691>] SYSC_sendto+0x121/0x1c0
[<ffffffff8109dd84>] ? vtime_account_user+0x54/0x60
[<ffffffff81020d35>] ? syscall_trace_enter+0x145/0x250
[<ffffffff8160319e>] SyS_sendto+0xe/0x10
[<ffffffff8172663f>] tracesys+0xe1/0xe6

Signed-Off-By: Ansis Atteka <aatteka at nicira.com>
---
 datapath/flow_netlink.c                        | 13 +++----------
 datapath/linux/compat/include/net/ip_tunnels.h |  1 +
 datapath/vport-geneve.c                        |  2 +-
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index e1eadbb..37646e4 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -421,6 +421,7 @@ static int ipv4_tun_from_nlattr(const struct nlattr *attr,
 			tun_flags |= TUNNEL_OAM;
 			break;
 		case OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS:
+			tun_flags |= TUNNEL_OPTIONS_PRESENT;
 			if (nla_len(a) > sizeof(match->key->tun_opts)) {
 				OVS_NLERR("Geneve option length exceeds "
 					  "maximum size (len %d, max %zu).\n",
@@ -1050,16 +1051,8 @@ int ovs_nla_put_flow(struct datapath *dp, const struct sw_flow_key *swkey,
 	if ((swkey->tun_key.ipv4_dst || is_mask)) {
 		const struct geneve_opt *opts = NULL;
 
-		if (!is_mask) {
-			struct vport *in_port;
-
-			in_port = ovs_vport_ovsl_rcu(dp, swkey->phy.in_port);
-			if (in_port->ops->type == OVS_VPORT_TYPE_GENEVE)
-				opts = GENEVE_OPTS(output, swkey->tun_opts_len);
-		} else {
-			if (output->tun_opts_len)
-				opts = GENEVE_OPTS(output, swkey->tun_opts_len);
-		}
+		if (output->tun_key.tun_flags & TUNNEL_OPTIONS_PRESENT)
+			opts = GENEVE_OPTS(output, swkey->tun_opts_len);
 
 		if (ipv4_tun_to_nlattr(skb, &output->tun_key, opts,
 					swkey->tun_opts_len))
diff --git a/datapath/linux/compat/include/net/ip_tunnels.h b/datapath/linux/compat/include/net/ip_tunnels.h
index c7a14ef..2a6470a 100644
--- a/datapath/linux/compat/include/net/ip_tunnels.h
+++ b/datapath/linux/compat/include/net/ip_tunnels.h
@@ -48,5 +48,6 @@ int iptunnel_pull_header(struct sk_buff *skb, int hdr_len, __be16 inner_proto);
 /* Not yet upstream */
 #define TUNNEL_OAM	__cpu_to_be16(0x0200)
 #define TUNNEL_CRIT_OPT	__cpu_to_be16(0x0400)
+#define TUNNEL_OPTIONS_PRESENT	__cpu_to_be16(0x0800)
 
 #endif /* __NET_IP_TUNNELS_H */
diff --git a/datapath/vport-geneve.c b/datapath/vport-geneve.c
index 99841d4..65a996f 100644
--- a/datapath/vport-geneve.c
+++ b/datapath/vport-geneve.c
@@ -189,7 +189,7 @@ static int geneve_rcv(struct sock *sk, struct sk_buff *skb)
 
 	geneveh = geneve_hdr(skb);
 
-	flags = TUNNEL_KEY |
+	flags = TUNNEL_KEY | TUNNEL_OPTIONS_PRESENT |
 		(udp_hdr(skb)->check != 0 ? TUNNEL_CSUM : 0) |
 		(geneveh->oam ? TUNNEL_OAM : 0) |
 		(geneveh->critical ? TUNNEL_CRIT_OPT : 0);
-- 
1.8.1.2




More information about the dev mailing list