From 182138487fed9deb53be95d78b3bd976c4a7e140 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 11 Nov 2008 15:35:26 -0800 Subject: [PATCH] Make datapath's flow_extract() properly pull data into the headers. Otherwise we might be reading headers that aren't really there. --- datapath/Modules.mk | 1 - datapath/flow.c | 122 ++++++++++-------- .../linux-2.4/compat-2.4/include/linux/ip.h | 5 + datapath/snap.h | 32 ----- 4 files changed, 73 insertions(+), 87 deletions(-) delete mode 100644 datapath/snap.h diff --git a/datapath/Modules.mk b/datapath/Modules.mk index 648e37f25..cbb52df83 100644 --- a/datapath/Modules.mk +++ b/datapath/Modules.mk @@ -28,7 +28,6 @@ openflow_headers = \ nx_act.h \ nx_act_snat.h \ nx_msg.h \ - snap.h \ table.h dist_sources = $(foreach module,$(dist_modules),$($(module)_sources)) diff --git a/datapath/flow.c b/datapath/flow.c index af9cdba77..48c2d4c9d 100644 --- a/datapath/flow.c +++ b/datapath/flow.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -22,7 +23,6 @@ #include "openflow/openflow.h" #include "compat.h" -#include "snap.h" struct kmem_cache *flow_cache; @@ -275,10 +275,40 @@ void print_flow(const struct sw_flow_key *key) } EXPORT_SYMBOL(print_flow); +#define SNAP_OUI_LEN 3 + +struct eth_snap_hdr +{ + struct ethhdr eth; + uint8_t dsap; /* Always 0xAA */ + uint8_t ssap; /* Always 0xAA */ + uint8_t ctrl; + uint8_t oui[SNAP_OUI_LEN]; + uint16_t ethertype; +} __attribute__ ((packed)); + +static int is_snap(const struct eth_snap_hdr *esh) +{ + return (esh->dsap == LLC_SAP_SNAP + && esh->ssap == LLC_SAP_SNAP + && !memcmp(esh->oui, "\0\0\0", 3)); +} + +static int iphdr_ok(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 0; +} + static int tcphdr_ok(struct sk_buff *skb) { int th_ofs = skb_transport_offset(skb); - if (skb->len >= th_ofs + sizeof(struct tcphdr)) { + if (pskb_may_pull(skb, th_ofs + sizeof(struct tcphdr))) { int tcp_len = tcp_hdrlen(skb); return (tcp_len >= sizeof(struct tcphdr) && skb->len >= th_ofs + tcp_len); @@ -289,7 +319,7 @@ static int tcphdr_ok(struct sk_buff *skb) static int udphdr_ok(struct sk_buff *skb) { int th_ofs = skb_transport_offset(skb); - return skb->len >= th_ofs + sizeof(struct udphdr); + return pskb_may_pull(skb, th_ofs + sizeof(struct udphdr)); } /* Parses the Ethernet frame in 'skb', which was received on 'in_port', @@ -298,58 +328,55 @@ static int udphdr_ok(struct sk_buff *skb) int flow_extract(struct sk_buff *skb, uint16_t in_port, struct sw_flow_key *key) { - struct ethhdr *mac; - int nh_ofs, th_ofs; + struct ethhdr *eth; + struct eth_snap_hdr *esh; int retval = 0; + int nh_ofs; + memset(key, 0, sizeof *key); + key->dl_vlan = htons(OFP_VLAN_NONE); key->in_port = htons(in_port); - key->pad = 0; - key->wildcards = 0; - key->nw_src_mask = 0; - key->nw_dst_mask = 0; - - /* This code doesn't check that skb->len is long enough to contain the - * MAC or network header. With a 46-byte minimum length frame this - * assumption is always correct. */ - - /* Doesn't verify checksums. Should it? */ - - /* Data link layer. We only support Ethernet. */ - mac = eth_hdr(skb); - nh_ofs = sizeof(struct ethhdr); - if (likely(ntohs(mac->h_proto) >= OFP_DL_TYPE_ETH2_CUTOFF)) { - /* This is an Ethernet II frame */ - key->dl_type = mac->h_proto; + + if (skb->len < sizeof *eth) + return 0; + if (!pskb_may_pull(skb, skb->len >= 64 ? 64 : skb->len)) { + return 0; + } + + eth = eth_hdr(skb); + esh = (struct eth_snap_hdr *) eth; + nh_ofs = sizeof *eth; + if (likely(ntohs(eth->h_proto) >= OFP_DL_TYPE_ETH2_CUTOFF)) + key->dl_type = eth->h_proto; + else if (skb->len >= sizeof *esh && is_snap(esh)) { + key->dl_type = esh->ethertype; + nh_ofs = sizeof *esh; } else { - /* This is an 802.2 frame */ - if (snap_get_ethertype(skb, &key->dl_type) != -EINVAL) { - nh_ofs += sizeof(struct snap_hdr); - } else { - key->dl_type = htons(OFP_DL_TYPE_NOT_ETH_TYPE); - nh_ofs += sizeof(struct llc_pdu_un); + key->dl_type = htons(OFP_DL_TYPE_NOT_ETH_TYPE); + if (skb->len >= nh_ofs + sizeof(struct llc_pdu_un)) { + nh_ofs += sizeof(struct llc_pdu_un); } } /* Check for a VLAN tag */ - if (likely(key->dl_type != htons(ETH_P_8021Q))) { - key->dl_vlan = htons(OFP_VLAN_NONE); - } else { - struct vlan_hdr *vh = (struct vlan_hdr *)(skb_mac_header(skb) + nh_ofs); + if (key->dl_type == htons(ETH_P_8021Q) && + skb->len >= nh_ofs + sizeof(struct vlan_hdr)) { + struct vlan_hdr *vh = (struct vlan_hdr*)(skb->data + nh_ofs); key->dl_type = vh->h_vlan_encapsulated_proto; key->dl_vlan = vh->h_vlan_TCI & htons(VLAN_VID_MASK); - nh_ofs += sizeof(*vh); + nh_ofs += sizeof(struct vlan_hdr); } - memcpy(key->dl_src, mac->h_source, ETH_ALEN); - memcpy(key->dl_dst, mac->h_dest, ETH_ALEN); + memcpy(key->dl_src, eth->h_source, ETH_ALEN); + memcpy(key->dl_dst, eth->h_dest, ETH_ALEN); skb_set_network_header(skb, nh_ofs); /* Network layer. */ - if (likely(key->dl_type == htons(ETH_P_IP))) { + if (key->dl_type == htons(ETH_P_IP) && iphdr_ok(skb)) { struct iphdr *nh = ip_hdr(skb); + int th_ofs = nh_ofs + nh->ihl * 4; key->nw_src = nh->saddr; key->nw_dst = nh->daddr; key->nw_proto = nh->protocol; - th_ofs = nh_ofs + nh->ihl * 4; skb_set_transport_header(skb, th_ofs); /* Transport layer. */ @@ -363,7 +390,7 @@ int flow_extract(struct sk_buff *skb, uint16_t in_port, /* Avoid tricking other code into * thinking that this packet has an L4 * header. */ - goto no_proto; + key->nw_proto = 0; } } else if (key->nw_proto == IPPROTO_UDP) { if (udphdr_ok(skb)) { @@ -374,28 +401,15 @@ int flow_extract(struct sk_buff *skb, uint16_t in_port, /* Avoid tricking other code into * thinking that this packet has an L4 * header. */ - goto no_proto; + key->nw_proto = 0; } - } else { - goto no_th; } } else { retval = 1; - goto no_th; } - - return 0; + } else { + skb_reset_transport_header(skb); } - - key->nw_src = 0; - key->nw_dst = 0; - -no_proto: - key->nw_proto = 0; - -no_th: - key->tp_src = 0; - key->tp_dst = 0; return retval; } diff --git a/datapath/linux-2.4/compat-2.4/include/linux/ip.h b/datapath/linux-2.4/compat-2.4/include/linux/ip.h index b2fbdb93f..990a57584 100644 --- a/datapath/linux-2.4/compat-2.4/include/linux/ip.h +++ b/datapath/linux-2.4/compat-2.4/include/linux/ip.h @@ -10,6 +10,11 @@ static inline struct iphdr *ip_hdr(const struct sk_buff *skb) { return (struct iphdr *)skb_network_header(skb); } + +static inline unsigned int ip_hdrlen(const struct sk_buff *skb) +{ + return ip_hdr(skb)->ihl * 4; +} #endif #endif diff --git a/datapath/snap.h b/datapath/snap.h deleted file mode 100644 index 0d39a6220..000000000 --- a/datapath/snap.h +++ /dev/null @@ -1,32 +0,0 @@ -#ifndef SNAP_H -#define SNAP_H 1 - -#include - -#define SNAP_OUI_LEN 3 - - -struct snap_hdr -{ - uint8_t dsap; /* Always 0xAA */ - uint8_t ssap; /* Always 0xAA */ - uint8_t ctrl; - uint8_t oui[SNAP_OUI_LEN]; - uint16_t ethertype; -} __attribute__ ((packed)); - -static inline int snap_get_ethertype(struct sk_buff *skb, uint16_t *ethertype) -{ - struct snap_hdr *sh = (struct snap_hdr *)(skb->data - + sizeof(struct ethhdr)); - if ((sh->dsap != LLC_SAP_SNAP) - || (sh->ssap != LLC_SAP_SNAP) - || (memcmp(sh->oui, "\0\0\0", SNAP_OUI_LEN))) - return -EINVAL; - - *ethertype = sh->ethertype; - - return 0; -} - -#endif /* snap.h */ -- 2.45.2