netdev - Re: [PATCH 2/3] net: UDP gro

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <52FC7D0C.1020601@mellanox.com>
Date:	Thu, 13 Feb 2014 10:06:36 +0200
From:	Or Gerlitz <ogerlitz@...lanox.com>
To:	Tom Herbert <therbert@...gle.com>, <davem@...emloft.net>,
	<netdev@...r.kernel.org>,
	Joseph Gasparakis <joseph.gasparakis@...el.com>
CC:	Jerry Chu <hkchu@...gle.com>, Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH 2/3] net: UDP gro_receive accept csum=0

On 11/02/2014 19:43, Tom Herbert wrote:
> The code to validate checksum in UDP gro_receive explictly checks
> against driver having set CHECKSUM_COMPLETE. This does not perform
> GRO on UDP packets with a checksum of zero (no checksum needed).
> This patch adds the condition to allow UDP checksum to be zero.
> Signed-off-by: Tom Herbert <therbert@...gle.com>
> ---
>   net/ipv4/udp_offload.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 25f5cee..4db7796 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -156,13 +156,9 @@ static struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *s
>   	unsigned int hlen, off;
>   	int flush = 1;
>   
> -	if (NAPI_GRO_CB(skb)->udp_mark ||
> -	    (!skb->encapsulation && skb->ip_summed != CHECKSUM_COMPLETE))
> +	if (NAPI_GRO_CB(skb)->udp_mark)
>   		goto out;
>   
> -	/* mark that this skb passed once through the udp gro layer */
> -	NAPI_GRO_CB(skb)->udp_mark = 1;
> -
>   	off  = skb_gro_offset(skb);
>   	hlen = off + sizeof(*uh);
>   	uh   = skb_gro_header_fast(skb, off);
> @@ -172,6 +168,13 @@ static struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *s
>   			goto out;
>   	}
>   
> +	if (!skb->encapsulation &&
> +	    skb->ip_summed != CHECKSUM_COMPLETE && uh->check != 0)
> +		goto out;
> +
> +	/* mark that this skb passed once through the udp gro layer */
> +	NAPI_GRO_CB(skb)->udp_mark = 1;
> +

Hi Tom,

Considering the patch just "as is" vs. the current code, its OK.

However, as skbs have only one indicator for the status of the checksum 
checks done by the receiving hardware, the basic assertion I thought we 
needed here is to reject skb if either it has the udp mark set or the 
encapsulation field is false, this is according to the conventions set 
by these two commits

0afb166 vxlan: Add capability of Rx checksum offload for inner packet
6a674e9 net: Add support for hardware-offloaded encapsulation

B/c after finalizing the GRO work and decapsulating, skb injected up 
into the TCP stack with ip_summed equals to CHECKSUM_NONE are rejected. 
If this assumption is wrong, maybe we can remove testing the ip_summed 
field here altogether?

Or.

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