netdev - RE: [PATCH net] Xen-netback: Fix issue caused by using gso

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]

Message-ID: <9AAE0902D5BC7E449B7C8E4E778ABCD0279FBD@AMSPEX01CL01.citrite.net>
Date:	Mon, 10 Mar 2014 09:51:26 +0000
From:	Paul Durrant <Paul.Durrant@...rix.com>
To:	Annie Li <Annie.li@...cle.com>,
	"xen-devel@...ts.xen.org" <xen-devel@...ts.xen.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:	"konrad.wilk@...cle.com" <konrad.wilk@...cle.com>,
	Ian Campbell <Ian.Campbell@...rix.com>,
	Wei Liu <wei.liu2@...rix.com>
Subject: RE: [PATCH net] Xen-netback: Fix issue caused by using gso_type
 wrongly

> -----Original Message-----
> From: Annie Li [mailto:Annie.li@...cle.com]
> Sent: 10 March 2014 07:44
> To: xen-devel@...ts.xen.org; netdev@...r.kernel.org
> Cc: konrad.wilk@...cle.com; Ian Campbell; Wei Liu; Paul Durrant;
> annie.li@...cle.com
> Subject: [PATCH net] Xen-netback: Fix issue caused by using gso_type
> wrongly
> 
> From: Annie Li <annie.li@...cle.com>
> 
> Current netback uses gso_type to check whether the skb contains
> gso offload, and this is wrong. Gso_size is the right one to
> check gso existence, and gso_type is only used to check gso type.
> 
> Some skbs contains nonzero gso_type and zero gso_size, current
> netback would treat these skbs as gso and create wrong response
> for this. This also causes ssh failure to domu from other server.
> 

That's a shame. It would be nice if these things were consistent...

> Signed-off-by: Annie Li <annie.li@...cle.com>
> ---
>  drivers/net/xen-netback/netback.c |   35 +++++++++++++++++---------------
> ---
>  1 files changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-
> netback/netback.c
> index e5284bc..57a7d5d 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -240,7 +240,7 @@ static void xenvif_gop_frag_copy(struct xenvif *vif,
> struct sk_buff *skb,
>  	struct gnttab_copy *copy_gop;
>  	struct xenvif_rx_meta *meta;
>  	unsigned long bytes;
> -	int gso_type;
> +	int gso_type = XEN_NETIF_GSO_TYPE_NONE;
> 
>  	/* Data must not cross a page boundary. */
>  	BUG_ON(size + offset > PAGE_SIZE<<compound_order(page));
> @@ -299,12 +299,12 @@ static void xenvif_gop_frag_copy(struct xenvif *vif,
> struct sk_buff *skb,
>  		}
> 
>  		/* Leave a gap for the GSO descriptor. */
> -		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
> -			gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
> -		else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
> -			gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
> -		else
> -			gso_type = XEN_NETIF_GSO_TYPE_NONE;
> +		if (skb_shinfo(skb)->gso_size) {

You should probably use skb_is_gso(skb) for your test.

  Paul

> +			if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
> +				gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
> +			else if (skb_shinfo(skb)->gso_type &
> SKB_GSO_TCPV6)
> +				gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
> +		}
> 
>  		if (*head && ((1 << gso_type) & vif->gso_mask))
>  			vif->rx.req_cons++;
> @@ -342,15 +342,13 @@ static int xenvif_gop_skb(struct sk_buff *skb,
> 
>  	old_meta_prod = npo->meta_prod;
> 
> -	if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) {
> -		gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
> -		gso_size = skb_shinfo(skb)->gso_size;
> -	} else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) {
> -		gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
> -		gso_size = skb_shinfo(skb)->gso_size;
> -	} else {
> -		gso_type = XEN_NETIF_GSO_TYPE_NONE;
> -		gso_size = 0;
> +	gso_type = XEN_NETIF_GSO_TYPE_NONE;
> +	gso_size = skb_shinfo(skb)->gso_size;
> +	if (gso_size) {
> +		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
> +			gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
> +		else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
> +			gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
>  	}
> 
>  	/* Set up a GSO prefix descriptor, if necessary */
> @@ -500,8 +498,9 @@ static void xenvif_rx_action(struct xenvif *vif)
>  			size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
>  			max_slots_needed += DIV_ROUND_UP(size,
> PAGE_SIZE);
>  		}
> -		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4 ||
> -		    skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
> +		if (skb_shinfo(skb)->gso_size &&
> +		   (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4 ||
> +		    skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6))
>  			max_slots_needed++;
> 
>  		/* If the skb may not fit then bail out now */
> --
> 1.7.3.4

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