From bf32fecdc1851ad9ca960f56771b798d17c26cf1 Mon Sep 17 00:00:00 2001 From: Jesse Gross Date: Mon, 2 Apr 2012 14:26:27 -0700 Subject: openvswitch: Add length check when retrieving TCP flags. When collecting TCP flags we check that the IP header indicates that a TCP header is present but not that the packet is actually long enough to contain the header. This adds a check to prevent reading off the end of the packet. In practice, this is only likely to result in reading of bad data and not a crash due to the presence of struct skb_shared_info at the end of the packet. Signed-off-by: Jesse Gross --- net/openvswitch/flow.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'net/openvswitch') diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c index 1252c3081ef1..2a11ec2383ee 100644 --- a/net/openvswitch/flow.c +++ b/net/openvswitch/flow.c @@ -183,7 +183,8 @@ void ovs_flow_used(struct sw_flow *flow, struct sk_buff *skb) u8 tcp_flags = 0; if (flow->key.eth.type == htons(ETH_P_IP) && - flow->key.ip.proto == IPPROTO_TCP) { + flow->key.ip.proto == IPPROTO_TCP && + likely(skb->len >= skb_transport_offset(skb) + sizeof(struct tcphdr))) { u8 *tcp = (u8 *)tcp_hdr(skb); tcp_flags = *(tcp + TCP_FLAGS_OFFSET) & TCP_FLAG_MASK; } -- cgit v1.2.3 From 4cb6e116bb97c8b87a1f4f95e99d0c8dda2a6e9b Mon Sep 17 00:00:00 2001 From: Ansis Atteka Date: Thu, 3 May 2012 18:40:38 -0700 Subject: openvswitch: Release rtnl_lock if ovs_vport_cmd_build_info() failed. This patch fixes a possible lock-up bug where rtnl_lock might not get released. Signed-off-by: Ansis Atteka Signed-off-by: Jesse Gross --- net/openvswitch/datapath.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'net/openvswitch') diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index e44e631ea952..4cb615d46363 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -1641,10 +1641,9 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info) reply = ovs_vport_cmd_build_info(vport, info->snd_pid, info->snd_seq, OVS_VPORT_CMD_NEW); if (IS_ERR(reply)) { - err = PTR_ERR(reply); netlink_set_err(init_net.genl_sock, 0, - ovs_dp_vport_multicast_group.id, err); - return 0; + ovs_dp_vport_multicast_group.id, PTR_ERR(reply)); + goto exit_unlock; } genl_notify(reply, genl_info_net(info), info->snd_pid, -- cgit v1.2.3 From 072ae6314a191e3a9fc309b1e4e539ac7abc48ad Mon Sep 17 00:00:00 2001 From: Pravin B Shelar Date: Mon, 7 May 2012 17:21:53 -0700 Subject: openvswitch: Validation of IPv6 set port action uses IPv4 header When the kernel validates set TCP/UDP port actions, it looks at the ports in the existing flow to make sure that the L4 header exists. However, these actions always use the IPv4 version of the struct. Following patch fixes this by checking for flow ip protocol first. Signed-off-by: Pravin B Shelar Signed-off-by: Jesse Gross --- net/openvswitch/datapath.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) (limited to 'net/openvswitch') diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index 4cb615d46363..777716bc80f7 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -421,6 +421,19 @@ static int validate_sample(const struct nlattr *attr, return validate_actions(actions, key, depth + 1); } +static int validate_tp_port(const struct sw_flow_key *flow_key) +{ + if (flow_key->eth.type == htons(ETH_P_IP)) { + if (flow_key->ipv4.tp.src && flow_key->ipv4.tp.dst) + return 0; + } else if (flow_key->eth.type == htons(ETH_P_IPV6)) { + if (flow_key->ipv6.tp.src && flow_key->ipv6.tp.dst) + return 0; + } + + return -EINVAL; +} + static int validate_set(const struct nlattr *a, const struct sw_flow_key *flow_key) { @@ -462,18 +475,13 @@ static int validate_set(const struct nlattr *a, if (flow_key->ip.proto != IPPROTO_TCP) return -EINVAL; - if (!flow_key->ipv4.tp.src || !flow_key->ipv4.tp.dst) - return -EINVAL; - - break; + return validate_tp_port(flow_key); case OVS_KEY_ATTR_UDP: if (flow_key->ip.proto != IPPROTO_UDP) return -EINVAL; - if (!flow_key->ipv4.tp.src || !flow_key->ipv4.tp.dst) - return -EINVAL; - break; + return validate_tp_port(flow_key); default: return -EINVAL; -- cgit v1.2.3 From 8aa51d64c1f526e43b1e7f89fb8b98c2fd583f4b Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Sun, 13 May 2012 08:44:18 +0000 Subject: openvswitch: checking wrong variable in queue_userspace_packet() "skb" is non-NULL here, for example we dereference it in skb_clone(). The intent was to test "nskb" which was just set. Signed-off-by: Dan Carpenter Acked-by: Jesse Gross Signed-off-by: David S. Miller --- net/openvswitch/datapath.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/openvswitch') diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index 777716bc80f7..e66341ec455c 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -321,7 +321,7 @@ static int queue_userspace_packet(int dp_ifindex, struct sk_buff *skb, return -ENOMEM; nskb = __vlan_put_tag(nskb, vlan_tx_tag_get(nskb)); - if (!skb) + if (!nskb) return -ENOMEM; nskb->vlan_tci = 0; -- cgit v1.2.3