From 4c1ad233122408d073dd2265e9998ee87036b5ef Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 27 Aug 2010 12:32:05 -0700 Subject: [PATCH] datapath: Report memory allocation errors in flow_extract(). Until now flow_extract() has simply returned a bogus flow when memory allocation errors occurred. This fixes the problem by propagating the error to the caller. Signed-off-by: Ben Pfaff --- datapath/datapath.c | 12 ++++++-- datapath/flow.c | 67 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 64 insertions(+), 15 deletions(-) diff --git a/datapath/datapath.c b/datapath/datapath.c index e8b2976e..a34049f9 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -547,11 +547,17 @@ void dp_process_received_packet(struct dp_port *p, struct sk_buff *skb) struct sw_flow *flow; struct sw_flow_actions *acts; struct loop_counter *loop; + int error; OVS_CB(skb)->dp_port = p; /* Extract flow from 'skb' into 'key'. */ - flow_extract(skb, p ? p->port_no : ODPP_NONE, &key); + error = flow_extract(skb, p ? p->port_no : ODPP_NONE, &key); + if (unlikely(error)) { + kfree_skb(skb); + return; + } + if (OVS_CB(skb)->is_frag && dp->drop_frags) { kfree_skb(skb); stats_counter_off = offsetof(struct dp_stats_percpu, n_frags); @@ -1359,7 +1365,9 @@ static int do_execute(struct datapath *dp, const struct odp_execute *execute) else skb->protocol = htons(ETH_P_802_2); - flow_extract(skb, execute->in_port, &key); + err = flow_extract(skb, execute->in_port, &key); + if (err) + goto error_free_skb; rcu_read_lock(); err = execute_actions(dp, skb, &key, actions->actions, diff --git a/datapath/flow.c b/datapath/flow.c index c37c8e0f..eae703d0 100644 --- a/datapath/flow.c +++ b/datapath/flow.c @@ -40,15 +40,27 @@ static inline bool arphdr_ok(struct sk_buff *skb) return pskb_may_pull(skb, nh_ofs + sizeof(struct arp_eth_header)); } -static inline bool iphdr_ok(struct sk_buff *skb) +static inline int check_iphdr(struct sk_buff *skb) { - int nh_ofs = skb_network_offset(skb); - if (skb->len >= nh_ofs + sizeof(struct iphdr)) { - int ip_len = ip_hdrlen(skb); - return (ip_len >= sizeof(struct iphdr) - && pskb_may_pull(skb, nh_ofs + ip_len)); - } - return false; + unsigned int nh_ofs = skb_network_offset(skb); + unsigned int ip_len; + + if (skb->len < nh_ofs + sizeof(struct iphdr)) + return -EINVAL; + + ip_len = ip_hdrlen(skb); + if (ip_len < sizeof(struct iphdr) || skb->len < nh_ofs + ip_len) + return -EINVAL; + + /* + * Pull enough header bytes to account for the IP header plus the + * longest transport header that we parse, currently 20 bytes for TCP. + */ + if (!pskb_may_pull(skb, min(nh_ofs + ip_len + 20, skb->len))) + return -ENOMEM; + + skb_set_transport_header(skb, nh_ofs + ip_len); + return 0; } static inline bool tcphdr_ok(struct sk_buff *skb) @@ -213,6 +225,8 @@ static __be16 parse_ethertype(struct sk_buff *skb) * * The caller must ensure that skb->len >= ETH_HLEN. * + * Returns 0 if successful, otherwise a negative errno value. + * * Sets OVS_CB(skb)->is_frag to %true if @skb is an IPv4 fragment, otherwise to * %false. */ @@ -226,8 +240,25 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct odp_flow_key *key) key->dl_vlan = htons(ODP_VLAN_NONE); OVS_CB(skb)->is_frag = false; + /* + * We would really like to pull as many bytes as we could possibly + * want to parse into the linear data area. Currently that is: + * + * 14 Ethernet header + * 4 VLAN header + * 60 max IP header with options + * 20 max TCP/UDP/ICMP header (don't care about options) + * -- + * 98 + * + * But Xen only allocates 64 or 72 bytes for the linear data area in + * netback, which means that we would reallocate and copy the skb's + * linear data on every packet if we did that. So instead just pull 64 + * bytes, which is always sufficient without IP options, and then check + * whether we need to pull more later when we look at the IP header. + */ if (!pskb_may_pull(skb, min(skb->len, 64u))) - return 0; + return -ENOMEM; skb_reset_mac_header(skb); @@ -245,14 +276,24 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct odp_flow_key *key) __skb_push(skb, skb->data - (unsigned char *)eth); /* Network layer. */ - if (key->dl_type == htons(ETH_P_IP) && iphdr_ok(skb)) { - struct iphdr *nh = ip_hdr(skb); - int th_ofs = skb_network_offset(skb) + nh->ihl * 4; + if (key->dl_type == htons(ETH_P_IP)) { + struct iphdr *nh; + int error; + + error = check_iphdr(skb); + if (unlikely(error)) { + if (error == -EINVAL) { + skb->transport_header = skb->network_header; + return 0; + } + return error; + } + + nh = ip_hdr(skb); key->nw_src = nh->saddr; key->nw_dst = nh->daddr; key->nw_tos = nh->tos & ~INET_ECN_MASK; key->nw_proto = nh->protocol; - skb_set_transport_header(skb, th_ofs); /* Transport layer. */ if (!(nh->frag_off & htons(IP_MF | IP_OFFSET))) { -- 2.30.2