netdev - [PATCH] net: Do not include padding in TCP GRO checksum

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20131115011413.7090.9349.stgit@ahduyck-fpga.jf.intel.com>
Date:	Thu, 14 Nov 2013 17:18:18 -0800
From:	Alexander Duyck <alexander.h.duyck@...el.com>
To:	davem@...emloft.net
Cc:	netdev@...r.kernel.org, edumazet@...gle.com,
	herbert@...dor.apana.org
Subject: [PATCH] net: Do not include padding in TCP GRO checksum

In some recent tests where I was generating invalid frames I found that
the checksum was being treated as valid for certain frames that computed
the checksum with padding included.  On closer inspection I found the
issue was that GRO was using the skb->len instead of the length recorded in
the IP/IPv6 header to determine the number of bytes to checksum.  As such
padded frames that actually had invalid checksums generated by adding the
padding to the checksum were being incorrectly tagged as valid.

This change corrects that by using the tot_len from IPv4 headers and the
payload_len from IPv6 headers to compute the total number of bytes to be
included in the checksum.

Signed-off-by: Alexander Duyck <alexander.h.duyck@...el.com>
---

I haven't had a chance to test this much yet and I am not that familiar
some of this GRO code so any review of this would be greatly appreciated.
I will try to get this tested by end-of-day tomorrow to verify it resolves
the issues I saw with invalid padded frames being marked as valid and
doesn't introduce any new issues.

 net/ipv4/tcp_offload.c   |   13 ++++++++-----
 net/ipv6/tcpv6_offload.c |   11 +++++++----
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index a2b68a1..3dabb76 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -273,13 +273,16 @@ static int tcp_v4_gso_send_check(struct sk_buff *skb)
 static struct sk_buff **tcp4_gro_receive(struct sk_buff **head, struct sk_buff *skb)
 {
 	const struct iphdr *iph = skb_gro_network_header(skb);
+	int length = iph->tot_len;
 	__wsum wsum;
 	__sum16 sum;
 
+	/* adjust for any offsets */
+	length += skb_network_offset(skb) - skb_gro_offset(skb);
+
 	switch (skb->ip_summed) {
 	case CHECKSUM_COMPLETE:
-		if (!tcp_v4_check(skb_gro_len(skb), iph->saddr, iph->daddr,
-				  skb->csum)) {
+		if (!tcp_v4_check(length, iph->saddr, iph->daddr, skb->csum)) {
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			break;
 		}
@@ -288,11 +291,11 @@ flush:
 		return NULL;
 
 	case CHECKSUM_NONE:
-		wsum = csum_tcpudp_nofold(iph->saddr, iph->daddr,
-					  skb_gro_len(skb), IPPROTO_TCP, 0);
+		wsum = csum_tcpudp_nofold(iph->saddr, iph->daddr, length,
+					  IPPROTO_TCP, 0);
 		sum = csum_fold(skb_checksum(skb,
 					     skb_gro_offset(skb),
-					     skb_gro_len(skb),
+					     length,
 					     wsum));
 		if (sum)
 			goto flush;
diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
index c1097c7..53fc71d 100644
--- a/net/ipv6/tcpv6_offload.c
+++ b/net/ipv6/tcpv6_offload.c
@@ -36,12 +36,16 @@ static struct sk_buff **tcp6_gro_receive(struct sk_buff **head,
 					 struct sk_buff *skb)
 {
 	const struct ipv6hdr *iph = skb_gro_network_header(skb);
+	int length = iph->payload_len;
 	__wsum wsum;
 	__sum16 sum;
 
+	/* adjust for any offset due to extension headers */
+	length += skb_transport_offset(skb) - skb_gro_offset(skb);
+
 	switch (skb->ip_summed) {
 	case CHECKSUM_COMPLETE:
-		if (!tcp_v6_check(skb_gro_len(skb), &iph->saddr, &iph->daddr,
+		if (!tcp_v6_check(length, &iph->saddr, &iph->daddr,
 				  skb->csum)) {
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 			break;
@@ -52,11 +56,10 @@ flush:
 
 	case CHECKSUM_NONE:
 		wsum = ~csum_unfold(csum_ipv6_magic(&iph->saddr, &iph->daddr,
-						    skb_gro_len(skb),
-						    IPPROTO_TCP, 0));
+						    length, IPPROTO_TCP, 0));
 		sum = csum_fold(skb_checksum(skb,
 					     skb_gro_offset(skb),
-					     skb_gro_len(skb),
+					     length,
 					     wsum));
 		if (sum)
 			goto flush;

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html