netdev - Re: [PATCH 1/4] pch_gbe: Fix the checksum fill to the error location

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Tue, 17 Jul 2012 09:09:28 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Andy Cress <andy.cress@...kontron.com>
Cc:	netdev@...r.kernel.org, Zhong Hongbo <hongbo.zhong@...driver.com>
Subject: Re: [PATCH 1/4] pch_gbe: Fix the checksum fill to the error
 location

On Mon, 2012-07-16 at 13:03 -0700, Andy Cress wrote:
> Author: Zhong Hongbo <hongbo.zhong@...driver.com>
> 
> Due to some unknown hardware limitations the pch_gbe hardware cannot
> calculate checksums when the length of network package is less
> than 64 bytes, where we will surprisingly encounter a problem of
> the destination IP incorrectly changed.
> 
> When forwarding network packages at the network layer the IP packages
> won't be relayed to the upper transport layer and analyzed there,
> consequently, skb->transport_header pointer will be mistakenly remained
> the same as that of skb->network_header, resulting in TCP checksum
> wrongly
> filled into the field of destination IP in IP header.
> 
> We can fix this issue by manually calculate the offset of the TCP
> checksum
>  and update it accordingly.
> 
> We would normally use the skb_checksum_start_offset(skb) here, but in
> this
> case it is sometimes -2 (csum_start=0 - skb_headroom=2 => -2), hence the
> manual calculation.
> 
> Signed-off-by: Zhong Hongbo <hongbo.zhong@...driver.com>
> Merged-by: Andy Cress <andy.cress@...kontron.com>


Hmm... I fail to understand why you care about NIC doing checksums,
while pch_gbe_tx_queue() make a _copy_ of each outgoing
packets.

There _must_ be a way to avoid most of these copies (ie not touching
payload), only mess with the header to insert these 2 nul bytes ?

/* [Header:14][payload] ---> [Header:14][paddong:2][payload]    */

So at device setup : dev->needed_headroom = 2;

and in xmit,

	if (skb_headroom(skb) < 2) {
		struct sk_buff *skb_new;

		skb_new = skb_realloc_headroom(skb, 2);
		if (!skb_new) { handle error }
		consume_skb(skb);
		skb = skb_new;
	}
	ptr = skb_push(skb, 2);
	memmove(ptr, ptr + 2, ETH_HLEN);
	ptr[ETH_HLEN] = 0;
	ptr[ETH_HLEN + 1] = 0;



--
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