Implement skb_partial_csum_set, for setting partial csums on untrusted packets.
Use it in virtio_net (replacing buggy version there), it's also going
to be used by TAP for partial csum support.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
drivers/net/virtio_net.c | 11 +----------
include/linux/skbuff.h | 1 +
net/core/skbuff.c | 29 +++++++++++++++++++++++++++++
3 files changed, 31 insertions(+), 10 deletions(-)
diff -r 72be3d596d31 include/linux/skbuff.h
--- a/include/linux/skbuff.h Wed Jan 09 15:57:40 2008 +1100
+++ b/include/linux/skbuff.h Wed Jan 09 16:56:41 2008 +1100
@@ -1804,5 +1804,6 @@ static inline void skb_forward_csum(stru
skb->ip_summed = CHECKSUM_NONE;
}
+bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off);
#endif /* __KERNEL__ */
#endif /* _LINUX_SKBUFF_H */
diff -r 72be3d596d31 net/core/skbuff.c
--- a/net/core/skbuff.c Wed Jan 09 15:57:40 2008 +1100
+++ b/net/core/skbuff.c Wed Jan 09 16:56:41 2008 +1100
@@ -2214,6 +2214,34 @@ int skb_cow_data(struct sk_buff *skb, in
return elt;
}
+/**
+ * skb_partial_csum_set - set up and verify partial csum values for packet
+ * @skb: the skb to set
+ * @start: the number of bytes after skb->data to start checksumming.
+ * @off: the offset from start to place the checksum.
+ *
+ * For untrusted partially-checksummed packets, we need to make sure the values
+ * for skb->csum_start and skb->csum_offset are valid so we don't
oops.
+ *
+ * This function checks and sets those values and skb->ip_summed: if this
+ * returns false you should drop the packet.
+ */
+bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off)
+{
+ if (unlikely(start > skb->len - 2) ||
+ unlikely((int)start + off > skb->len - 2)) {
+ if (net_ratelimit())
+ printk(KERN_WARNING
+ "bad partial csum: csum=%u/%u len=%u\n",
+ start, off, skb->len);
+ return false;
+ }
+ skb->ip_summed = CHECKSUM_PARTIAL;
+ skb->csum_start = skb_headroom(skb) + start;
+ skb->csum_offset = off;
+ return true;
+}
+
EXPORT_SYMBOL(___pskb_trim);
EXPORT_SYMBOL(__kfree_skb);
EXPORT_SYMBOL(kfree_skb);
@@ -2250,3 +2278,4 @@ EXPORT_SYMBOL(skb_append_datato_frags);
EXPORT_SYMBOL_GPL(skb_to_sgvec);
EXPORT_SYMBOL_GPL(skb_cow_data);
+EXPORT_SYMBOL_GPL(skb_partial_csum_set);
diff -r 72be3d596d31 drivers/net/virtio_net.c
--- a/drivers/net/virtio_net.c Wed Jan 09 15:57:40 2008 +1100
+++ b/drivers/net/virtio_net.c Wed Jan 09 16:56:41 2008 +1100
@@ -89,17 +89,8 @@ static void receive_skb(struct net_devic
if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
pr_debug("Needs csum!\n");
- skb->ip_summed = CHECKSUM_PARTIAL;
- skb->csum_start = hdr->csum_start;
- skb->csum_offset = hdr->csum_offset;
- if (skb->csum_start > skb->len - 2
- || skb->csum_offset > skb->len - 2) {
- if (net_ratelimit())
- printk(KERN_WARNING "%s: csum=%u/%u len=%u\n",
- dev->name, skb->csum_start,
- skb->csum_offset, skb->len);
+ if (!skb_partial_csum_set(skb,hdr->csum_start,hdr->csum_offset))
goto frame_err;
- }
}
if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
It's far easier to deal with GSO if we don't have to parse the packet
to figure out the header length. Add the field to the virtio_net_hdr
struct (and fix the spaces that somehow crept in there).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
drivers/net/virtio_net.c | 4 +++-
include/linux/virtio_net.h | 11 ++++++-----
2 files changed, 9 insertions(+), 6 deletions(-)
diff -r 24ef33a4ab14 drivers/net/virtio_net.c
--- a/drivers/net/virtio_net.c Tue Jan 15 16:59:58 2008 +1100
+++ b/drivers/net/virtio_net.c Tue Jan 15 21:21:40 2008 +1100
@@ -126,6 +126,7 @@ static void receive_skb(struct net_devic
/* Header must be checked, and gso_segs computed. */
skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
skb_shinfo(skb)->gso_segs = 0;
+ skb_set_transport_header(skb, hdr->gso_hdr_len);
}
netif_receive_skb(skb);
@@ -247,6 +248,7 @@ static int start_xmit(struct sk_buff *sk
}
if (skb_is_gso(skb)) {
+ hdr->gso_hdr_len = skb_transport_header(skb) - skb->data;
hdr->gso_size = skb_shinfo(skb)->gso_size;
if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4_ECN;
@@ -260,7 +262,7 @@ static int start_xmit(struct sk_buff *sk
BUG();
} else {
hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
- hdr->gso_size = 0;
+ hdr->gso_size = hdr->gso_hdr_len = 0;
}
vnet_hdr_to_sg(sg, skb);
diff -r 24ef33a4ab14 include/linux/virtio_net.h
--- a/include/linux/virtio_net.h Tue Jan 15 16:59:58 2008 +1100
+++ b/include/linux/virtio_net.h Tue Jan 15 21:21:40 2008 +1100
@@ -24,16 +24,17 @@ struct virtio_net_hdr
struct virtio_net_hdr
{
#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 // Use csum_start, csum_offset
- __u8 flags;
+ __u8 flags;
#define VIRTIO_NET_HDR_GSO_NONE 0 // Not a GSO frame
#define VIRTIO_NET_HDR_GSO_TCPV4 1 // GSO frame, IPv4 TCP (TSO)
/* FIXME: Do we need this? If they said they can handle ECN, do they care? */
#define VIRTIO_NET_HDR_GSO_TCPV4_ECN 2 // GSO frame, IPv4 TCP w/ ECN
#define VIRTIO_NET_HDR_GSO_UDP 3 // GSO frame, IPv4 UDP (UFO)
#define VIRTIO_NET_HDR_GSO_TCPV6 4 // GSO frame, IPv6 TCP
- __u8 gso_type;
- __u16 gso_size;
- __u16 csum_start;
- __u16 csum_offset;
+ __u8 gso_type;
+ __u16 gso_hdr_len; /* Ethernet + IP + tcp/udp hdrs */
+ __u16 gso_size; /* Bytes to append to gso_hdr_len per frame */
+ __u16 csum_start; /* Position to start checksumming from */
+ __u16 csum_offset; /* Offset after that to place checksum */
};
#endif /* _LINUX_VIRTIO_NET_H */
From: Rusty Russell <rusty@rustcorp.com.au> Date: Tue, 15 Jan 2008 21:41:55 +1100> Implement skb_partial_csum_set, for setting partial csums on untrusted packets. > > Use it in virtio_net (replacing buggy version there), it's also going > to be used by TAP for partial csum support. > > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>Looks fine to me. Acked-by: David S. Miller <davem@davemloft.net> If you like I can merge this into my net-2.6.25 tree, or alternatively if it makes your life easier you then you can handle it yourself.