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]
Date:	Tue, 12 Nov 2013 17:13:43 -0800
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Herbert Xu <herbert@...dor.apana.org.au>
Cc:	Ben Hutchings <bhutchings@...arflare.com>,
	David Miller <davem@...emloft.net>,
	christoph.paasch@...ouvain.be, netdev@...r.kernel.org,
	hkchu@...gle.com, mwdalton@...gle.com
Subject: Re: gso: Handle new frag_list of frags GRO packets

On Tue, 2013-11-12 at 02:52 +0800, Herbert Xu wrote:
> Recently GRO started generating packets with frag_lists of frags.
> This was not handled by GSO, thus leading to a crash.
> 
> Thankfully these packets are of a regular form and are easy to
> handle.  This patch handles them in two ways.  For completely
> non-linear frag_list entries, we simply continue to iterate over
> the frag_list frags once we exhaust the normal frags.  For frag_list
> entries with linear parts, we call pskb_trim on the first part
> of the frag_list skb, and then process the rest of the frags in
> the usual way.
> 
> This patch also kills a chunk of dead frag_list code that has
> obviously never ever been run since it ends up generating a bogus
> GSO-segmented packet with a frag_list entry.
> 
> Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>
> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 3735fad..557e1a5 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2776,6 +2776,7 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
>  	struct sk_buff *segs = NULL;
>  	struct sk_buff *tail = NULL;
>  	struct sk_buff *fskb = skb_shinfo(skb)->frag_list;
> +	skb_frag_t *skb_frag = skb_shinfo(skb)->frags;
>  	unsigned int mss = skb_shinfo(skb)->gso_size;
>  	unsigned int doffset = skb->data - skb_mac_header(skb);
>  	unsigned int offset = doffset;
> @@ -2815,16 +2816,38 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
>  		if (hsize > len || !sg)
>  			hsize = len;
>  
> -		if (!hsize && i >= nfrags) {
> -			BUG_ON(fskb->len != len);
> +		if (!hsize && i >= nfrags && skb_headlen(fskb) &&
> +		    (skb_headlen(fskb) == len || sg)) {
> +			BUG_ON(skb_headlen(fskb) > len);
> +

Hmm, yet another BUG_ON() case...

> +			i = 0;
> +			nfrags = skb_shinfo(fskb)->nr_frags;
> +			skb_frag = skb_shinfo(fskb)->frags;
> +			pos += skb_headlen(fskb);
> +
> +			while (pos < offset + len) {
> +				BUG_ON(i >= nfrags);
> +
> +				size = skb_frag_size(skb_frag);
> +				if (pos + size > offset + len)
> +					break;
> +
> +				i++;
> +				pos += size;
> +				skb_frag++;
> +			}
>  
> -			pos += len;
>  			nskb = skb_clone(fskb, GFP_ATOMIC);
>  			fskb = fskb->next;
>  
>  			if (unlikely(!nskb))
>  				goto err;
>  
> +			if (unlikely(pskb_trim(nskb, len))) {
> +				kfree_skb(nskb);
> +				goto err;
> +			}
> +

Note this pskb_trim() will reallocate/copy nskb head completely, since
nskb is a clone. (And increment page counts of frags, then eventually
decrement them)

I tested this patch one 'router', and it seems fine, although we consume
~90% more cpu doing the skb_segment() than not doing it.


GRO not building frag_list skbs :

lpaa6:~# mpstat -P 8 1 10
05:09:47 PM  CPU    %usr   %nice    %sys %iowait    %irq   %soft  %steal  %guest   %idle    intr/s
05:09:48 PM    8    0.00    0.00    1.01    0.00    3.03   23.23    0.00    0.00   72.73  43462.63
05:09:49 PM    8    0.00    0.00    0.00    0.00    5.10   28.57    0.00    0.00   66.33  88079.59
05:09:50 PM    8    0.00    0.00    0.00    0.00    2.13   17.02    0.00    0.00   80.85  41297.87
05:09:51 PM    8    0.00    0.00    0.95    0.00    3.81   29.52    0.00    0.00   65.71  45741.90
05:09:52 PM    8    0.00    0.00    0.00    0.00    2.11   17.89    0.00    0.00   80.00  25413.68
05:09:53 PM    8    1.03    0.00    1.03    0.00    2.06   20.62    0.00    0.00   75.26  36131.96
05:09:54 PM    8    0.00    0.00    0.94    0.00    3.77   30.19    0.00    0.00   65.09  47100.00
05:09:55 PM    8    0.00    0.00    0.00    0.00    3.26   21.74    0.00    0.00   75.00  71805.43
05:09:56 PM    8    0.00    0.00    0.00    0.00    3.19   22.34    0.00    0.00   74.47  70672.34
05:09:57 PM    8    0.00    0.00    0.00    0.00    4.50   32.43    0.00    0.00   63.06  45919.82
Average:       8    0.10    0.00    0.40    0.00    3.33   24.62    0.00    0.00   71.54  51339.66


Current GRO (large skbs -> need to split them with skb_segment(), no TSO is used)

lpaa6:~# mpstat -P 8 1 10
05:10:05 PM  CPU    %usr   %nice    %sys %iowait    %irq   %soft  %steal  %guest   %idle    intr/s
05:10:06 PM    8    0.00    0.00    0.00    0.00    8.16   44.90    0.00    0.00   46.94  88982.65
05:10:07 PM    8    0.00    0.00    1.05    0.00    8.42   50.53    0.00    0.00   40.00  54796.84
05:10:08 PM    8    0.00    0.00    1.94    0.00    8.74   52.43    0.00    0.00   36.89  50163.11
05:10:09 PM    8    0.00    0.00    0.00    0.00    7.14   39.80    0.00    0.00   53.06  85137.76
05:10:10 PM    8    0.00    0.00    0.00    0.00    8.08   44.44    0.00    0.00   47.47  42262.63
05:10:11 PM    8    0.00    0.00    0.00    0.00    8.00   53.00    0.00    0.00   39.00  53444.00
05:10:12 PM    8    0.00    0.00    0.00    0.00    5.00   27.50    0.00    0.00   67.50  91098.75
05:10:13 PM    8    0.00    0.00    0.00    0.00    8.55   47.86    0.00    0.00   43.59  34316.24
05:10:14 PM    8    0.00    0.00    0.00    0.00    8.70   48.91    0.00    0.00   42.39  56921.74
05:10:15 PM    8    0.00    0.00    0.93    0.00    7.41   46.30    0.00    0.00   45.37  77129.63
Average:       8    0.00    0.00    0.40    0.00    7.88   45.96    0.00    0.00   45.76  62458.99



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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ