Some of the flow actions that modify skbuff data did not check that the
skbuff was long enough before doing so. This commit fixes that problem.
Previously, the strategy for avoiding this was to only indicate the layer-3
nw_proto field in the flow if the corresponding layer-4 header was fully
present, so that if, for example, nw_proto was IPPROTO_TCP, this meant
that a TCP header was present. The original motivation for this patch was
to add corresponding code to only indicate a layer-2 dl_type if the
corresponding layer-3 header was fully present. But I'm now convinced that
this approach is conceptually wrong, because the meaning of a layer-N
header should not be affected by the meaning of a layer-(N+1) header.
This commit switches to a new approach. Now, when a header is missing, its
fields in the flow are simply zeroed and have no effect on the "type" field
for the outer header. Responsibility for ensuring that a header is fully
present is now shifted to the actions that wish to modify that header.
Signed-off-by: Ben Pfaff <blp@nicira.com>
struct ethhdr *eh;
/* Verify we were given a vlan packet */
struct ethhdr *eh;
/* Verify we were given a vlan packet */
- if (vh->h_vlan_proto != htons(ETH_P_8021Q))
+ if (vh->h_vlan_proto != htons(ETH_P_8021Q) || skb->len < VLAN_ETH_HLEN)
return skb;
if (OVS_CB(skb)->ip_summed == OVS_CSUM_COMPLETE)
return skb;
if (OVS_CB(skb)->ip_summed == OVS_CSUM_COMPLETE)
if (skb->protocol == htons(ETH_P_8021Q)) {
/* Modify vlan id, but maintain other TCI values */
if (skb->protocol == htons(ETH_P_8021Q)) {
/* Modify vlan id, but maintain other TCI values */
- struct vlan_ethhdr *vh = vlan_eth_hdr(skb);
- __be16 old_tci = vh->h_vlan_TCI;
+ struct vlan_ethhdr *vh;
+ __be16 old_tci;
+
+ if (skb->len < VLAN_ETH_HLEN)
+ return skb;
+
+ vh = vlan_eth_hdr(skb);
+ old_tci = vh->h_vlan_TCI;
vh->h_vlan_TCI = htons((ntohs(vh->h_vlan_TCI) & ~mask) | tci);
vh->h_vlan_TCI = htons((ntohs(vh->h_vlan_TCI) & ~mask) | tci);
+static bool is_ip(struct sk_buff *skb, const struct odp_flow_key *key)
+{
+ return (key->dl_type == htons(ETH_P_IP) &&
+ skb->transport_header > skb->network_header);
+}
+
+static __sum16 *get_l4_checksum(struct sk_buff *skb, const struct odp_flow_key *key)
+{
+ int transport_len = skb->len - skb_transport_offset(skb);
+ if (key->nw_proto == IPPROTO_TCP) {
+ if (likely(transport_len >= sizeof(struct tcphdr)))
+ return &tcp_hdr(skb)->check;
+ } else if (key->nw_proto == IPPROTO_UDP) {
+ if (likely(transport_len >= sizeof(struct udphdr)))
+ return &udp_hdr(skb)->check;
+ }
+ return NULL;
+}
+
static struct sk_buff *set_nw_addr(struct sk_buff *skb,
const struct odp_flow_key *key,
const struct odp_action_nw_addr *a,
gfp_t gfp)
{
static struct sk_buff *set_nw_addr(struct sk_buff *skb,
const struct odp_flow_key *key,
const struct odp_action_nw_addr *a,
gfp_t gfp)
{
- if (key->dl_type != htons(ETH_P_IP))
+ struct iphdr *nh;
+ __sum16 *check;
+ __be32 *nwaddr;
+
+ if (unlikely(!is_ip(skb, key)))
return skb;
skb = make_writable(skb, 0, gfp);
return skb;
skb = make_writable(skb, 0, gfp);
- if (skb) {
- struct iphdr *nh = ip_hdr(skb);
- u32 *f = a->type == ODPAT_SET_NW_SRC ? &nh->saddr : &nh->daddr;
- u32 old = *f;
- u32 new = a->nw_addr;
-
- if (key->nw_proto == IPPROTO_TCP) {
- struct tcphdr *th = tcp_hdr(skb);
- update_csum(&th->check, skb, old, new, 1);
- } else if (key->nw_proto == IPPROTO_UDP) {
- struct udphdr *th = udp_hdr(skb);
- update_csum(&th->check, skb, old, new, 1);
- }
- update_csum(&nh->check, skb, old, new, 0);
- *f = new;
- }
+ if (unlikely(!skb))
+ return NULL;
+
+ nh = ip_hdr(skb);
+ nwaddr = a->type == ODPAT_SET_NW_SRC ? &nh->saddr : &nh->daddr;
+
+ check = get_l4_checksum(skb, key);
+ if (likely(check))
+ update_csum(check, skb, *nwaddr, a->nw_addr, 1);
+ update_csum(&nh->check, skb, *nwaddr, a->nw_addr, 0);
+
+ *nwaddr = a->nw_addr;
+
const struct odp_action_nw_tos *a,
gfp_t gfp)
{
const struct odp_action_nw_tos *a,
gfp_t gfp)
{
- if (key->dl_type != htons(ETH_P_IP))
+ if (unlikely(!is_ip(skb, key)))
return skb;
skb = make_writable(skb, 0, gfp);
return skb;
skb = make_writable(skb, 0, gfp);
/* Set the DSCP bits and preserve the ECN bits. */
new = a->nw_tos | (nh->tos & INET_ECN_MASK);
/* Set the DSCP bits and preserve the ECN bits. */
new = a->nw_tos | (nh->tos & INET_ECN_MASK);
- update_csum(&nh->check, skb, htons((uint16_t)old),
- htons((uint16_t)new), 0);
+ update_csum(&nh->check, skb, htons((u16)old),
+ htons((u16)new), 0);
const struct odp_flow_key *key,
const struct odp_action_tp_port *a, gfp_t gfp)
{
const struct odp_flow_key *key,
const struct odp_action_tp_port *a, gfp_t gfp)
{
+ struct udphdr *th;
+ __sum16 *check;
+ __be16 *port;
- if (key->dl_type != htons(ETH_P_IP))
+ if (unlikely(!is_ip(skb, key)))
- if (key->nw_proto == IPPROTO_TCP)
- check_ofs = offsetof(struct tcphdr, check);
- else if (key->nw_proto == IPPROTO_UDP)
- check_ofs = offsetof(struct udphdr, check);
- else
+ skb = make_writable(skb, 0, gfp);
+ if (unlikely(!skb))
+ return NULL;
+
+ /* Must follow make_writable() since that can move the skb data. */
+ check = get_l4_checksum(skb, key);
+ if (unlikely(!check))
- skb = make_writable(skb, 0, gfp);
- if (skb) {
- struct udphdr *th = udp_hdr(skb);
- u16 *f = a->type == ODPAT_SET_TP_SRC ? &th->source : &th->dest;
- u16 old = *f;
- u16 new = a->tp_port;
- update_csum((u16*)(skb_transport_header(skb) + check_ofs),
- skb, old, new, 0);
- *f = new;
- }
+ /*
+ * Update port and checksum.
+ *
+ * This is OK because source and destination port numbers are at the
+ * same offsets in both UDP and TCP headers, and get_l4_checksum() only
+ * supports those protocols.
+ */
+ th = udp_hdr(skb);
+ port = a->type == ODPAT_SET_TP_SRC ? &th->source : &th->dest;
+ update_csum(check, skb, *port, a->tp_port, 0);
+ *port = a->tp_port;
+
struct tcphdr *tcp = tcp_hdr(skb);
key->tp_src = tcp->source;
key->tp_dst = tcp->dest;
struct tcphdr *tcp = tcp_hdr(skb);
key->tp_src = tcp->source;
key->tp_dst = tcp->dest;
- } else {
- /* Avoid tricking other code into
- * thinking that this packet has an L4
- * header. */
- key->nw_proto = 0;
}
} else if (key->nw_proto == IPPROTO_UDP) {
if (udphdr_ok(skb)) {
struct udphdr *udp = udp_hdr(skb);
key->tp_src = udp->source;
key->tp_dst = udp->dest;
}
} else if (key->nw_proto == IPPROTO_UDP) {
if (udphdr_ok(skb)) {
struct udphdr *udp = udp_hdr(skb);
key->tp_src = udp->source;
key->tp_dst = udp->dest;
- } else {
- /* Avoid tricking other code into
- * thinking that this packet has an L4
- * header. */
- key->nw_proto = 0;
}
} else if (key->nw_proto == IPPROTO_ICMP) {
if (icmphdr_ok(skb)) {
}
} else if (key->nw_proto == IPPROTO_ICMP) {
if (icmphdr_ok(skb)) {
* in 16-bit network byte order. */
key->tp_src = htons(icmp->type);
key->tp_dst = htons(icmp->code);
* in 16-bit network byte order. */
key->tp_src = htons(icmp->type);
key->tp_dst = htons(icmp->code);
- } else {
- /* Avoid tricking other code into
- * thinking that this packet has an L4
- * header. */
- key->nw_proto = 0;
memcpy(eh->eth_dst, dl_addr, sizeof eh->eth_dst);
}
memcpy(eh->eth_dst, dl_addr, sizeof eh->eth_dst);
}
+static bool
+is_ip(const struct ofpbuf *packet, const flow_t *key)
+{
+ return key->dl_type == htons(ETH_TYPE_IP) && packet->l4;
+}
+
static void
dp_netdev_set_nw_addr(struct ofpbuf *packet, const flow_t *key,
const struct odp_action_nw_addr *a)
{
static void
dp_netdev_set_nw_addr(struct ofpbuf *packet, const flow_t *key,
const struct odp_action_nw_addr *a)
{
- if (key->dl_type == htons(ETH_TYPE_IP)) {
+ if (is_ip(packet, key)) {
struct ip_header *nh = packet->l3;
uint32_t *field;
field = a->type == ODPAT_SET_NW_SRC ? &nh->ip_src : &nh->ip_dst;
struct ip_header *nh = packet->l3;
uint32_t *field;
field = a->type == ODPAT_SET_NW_SRC ? &nh->ip_src : &nh->ip_dst;
- if (key->nw_proto == IP_TYPE_TCP) {
+ if (key->nw_proto == IP_TYPE_TCP && packet->l7) {
struct tcp_header *th = packet->l4;
th->tcp_csum = recalc_csum32(th->tcp_csum, *field, a->nw_addr);
struct tcp_header *th = packet->l4;
th->tcp_csum = recalc_csum32(th->tcp_csum, *field, a->nw_addr);
- } else if (key->nw_proto == IP_TYPE_UDP) {
+ } else if (key->nw_proto == IP_TYPE_UDP && packet->l7) {
struct udp_header *uh = packet->l4;
if (uh->udp_csum) {
uh->udp_csum = recalc_csum32(uh->udp_csum, *field, a->nw_addr);
struct udp_header *uh = packet->l4;
if (uh->udp_csum) {
uh->udp_csum = recalc_csum32(uh->udp_csum, *field, a->nw_addr);
dp_netdev_set_nw_tos(struct ofpbuf *packet, const flow_t *key,
const struct odp_action_nw_tos *a)
{
dp_netdev_set_nw_tos(struct ofpbuf *packet, const flow_t *key,
const struct odp_action_nw_tos *a)
{
- if (key->dl_type == htons(ETH_TYPE_IP)) {
+ if (is_ip(packet, key)) {
struct ip_header *nh = packet->l3;
uint8_t *field = &nh->ip_tos;
struct ip_header *nh = packet->l3;
uint8_t *field = &nh->ip_tos;
dp_netdev_set_tp_port(struct ofpbuf *packet, const flow_t *key,
const struct odp_action_tp_port *a)
{
dp_netdev_set_tp_port(struct ofpbuf *packet, const flow_t *key,
const struct odp_action_tp_port *a)
{
- if (key->dl_type == htons(ETH_TYPE_IP)) {
+ if (is_ip(packet, key)) {
- if (key->nw_proto == IPPROTO_TCP) {
+ if (key->nw_proto == IPPROTO_TCP && packet->l7) {
struct tcp_header *th = packet->l4;
field = a->type == ODPAT_SET_TP_SRC ? &th->tcp_src : &th->tcp_dst;
th->tcp_csum = recalc_csum16(th->tcp_csum, *field, a->tp_port);
*field = a->tp_port;
struct tcp_header *th = packet->l4;
field = a->type == ODPAT_SET_TP_SRC ? &th->tcp_src : &th->tcp_dst;
th->tcp_csum = recalc_csum16(th->tcp_csum, *field, a->tp_port);
*field = a->tp_port;
- } else if (key->nw_proto == IPPROTO_UDP) {
+ } else if (key->nw_proto == IPPROTO_UDP && packet->l7) {
struct udp_header *uh = packet->l4;
field = a->type == ODPAT_SET_TP_SRC ? &uh->udp_src : &uh->udp_dst;
uh->udp_csum = recalc_csum16(uh->udp_csum, *field, a->tp_port);
struct udp_header *uh = packet->l4;
field = a->type == ODPAT_SET_TP_SRC ? &uh->udp_src : &uh->udp_dst;
uh->udp_csum = recalc_csum16(uh->udp_csum, *field, a->tp_port);
return llc->snap.snap_type;
}
return llc->snap.snap_type;
}
-/* Returns 1 if 'packet' is an IP fragment, 0 otherwise.
- * 'tun_id' is in network byte order, while 'in_port' is in host byte order.
- * These byte orders are the same as they are in struct odp_flow_key. */
+/* 'tun_id' is in network byte order, while 'in_port' is in host byte order.
+ * These byte orders are the same as they are in struct odp_flow_key.
+ *
+ * Initializes packet header pointers as follows:
+ *
+ * - packet->l2 to the start of the Ethernet header.
+ *
+ * - packet->l3 to just past the Ethernet header, or just past the
+ * vlan_header if one is present, to the first byte of the payload of the
+ * Ethernet frame.
+ *
+ * - packet->l4 to just past the IPv4 header, if one is present and has a
+ * correct length, and otherwise NULL.
+ *
+ * - packet->l7 to just past the TCP or UDP or ICMP header, if one is
+ * present and has a correct length, and otherwise NULL.
+ */
int
flow_extract(struct ofpbuf *packet, uint32_t tun_id, uint16_t in_port,
flow_t *flow)
int
flow_extract(struct ofpbuf *packet, uint32_t tun_id, uint16_t in_port,
flow_t *flow)
flow->tp_src = tcp->tcp_src;
flow->tp_dst = tcp->tcp_dst;
packet->l7 = b.data;
flow->tp_src = tcp->tcp_src;
flow->tp_dst = tcp->tcp_dst;
packet->l7 = b.data;
- } else {
- /* Avoid tricking other code into thinking that
- * this packet has an L4 header. */
- flow->nw_proto = 0;
}
} else if (flow->nw_proto == IP_TYPE_UDP) {
const struct udp_header *udp = pull_udp(&b);
}
} else if (flow->nw_proto == IP_TYPE_UDP) {
const struct udp_header *udp = pull_udp(&b);
flow->tp_src = udp->udp_src;
flow->tp_dst = udp->udp_dst;
packet->l7 = b.data;
flow->tp_src = udp->udp_src;
flow->tp_dst = udp->udp_dst;
packet->l7 = b.data;
- } else {
- /* Avoid tricking other code into thinking that
- * this packet has an L4 header. */
- flow->nw_proto = 0;
}
} else if (flow->nw_proto == IP_TYPE_ICMP) {
const struct icmp_header *icmp = pull_icmp(&b);
}
} else if (flow->nw_proto == IP_TYPE_ICMP) {
const struct icmp_header *icmp = pull_icmp(&b);
flow->icmp_type = htons(icmp->icmp_type);
flow->icmp_code = htons(icmp->icmp_code);
packet->l7 = b.data;
flow->icmp_type = htons(icmp->icmp_type);
flow->icmp_code = htons(icmp->icmp_code);
packet->l7 = b.data;
- } else {
- /* Avoid tricking other code into thinking that
- * this packet has an L4 header. */
- flow->nw_proto = 0;