Re: Bug in skb_segment: fskb->len != len

Christoph Paasch
2013-10-29T09:09:02Z

On 28/10/13 - 18:15:08, Eric Dumazet wrote:
> On Mon, 2013-10-28 at 06:21 -0700, Eric Dumazet wrote:
>
> > But we also need to fix the skb_segment() bug anyway.
>
> Hi Christoph
>
> I cooked a minimal patch, could you please try it ?
>
> I'll refactor skb_segment() to be smarter for the next release
> (linux-3.14).
>
> Thanks !
>
> net/core/skbuff.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)

Ok, my router does not crash anymore with my workload.

Thanks for fixing it!

Tested-by: Christoph Paasch <christoph.paasch@uclouvain.be>

Cheers,
Christoph

>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 0ab32faa520f..771946487a8d 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2761,7 +2761,7 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> unsigned int len;
> __be16 proto;
> bool csum;
> - int sg = !!(features & NETIF_F_SG);
> + bool sg = !!(features & NETIF_F_SG);
> int nfrags = skb_shinfo(skb)->nr_frags;
> int err = -ENOMEM;
> int i = 0;
> @@ -2793,7 +2793,11 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> hsize = len;
>
> if (!hsize && i >= nfrags) {
> - BUG_ON(fskb->len != len);
> + if (fskb->len != len) {
> + hsize = len;
> + sg = false;
> + goto do_linear;
> + }
>
> pos += len;
> nskb = skb_clone(fskb, GFP_ATOMIC);
> @@ -2812,6 +2816,7 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> skb_release_head_state(nskb);
> __skb_push(nskb, doffset);
> } else {
> +do_linear:
> nskb = __alloc_skb(hsize + doffset + headroom,
> GFP_ATOMIC, skb_alloc_rx_flag(skb),
> NUMA_NO_NODE);
> @@ -2838,9 +2843,6 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> nskb->data - tnl_hlen,
> doffset + tnl_hlen);
>
> - if (fskb != skb_shinfo(skb)->frag_list)
> - goto perform_csum_check;
> -
> if (!sg) {
> nskb->ip_summed = CHECKSUM_NONE;
> nskb->csum = skb_copy_and_csum_bits(skb, offset,
> @@ -2849,6 +2851,9 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> continue;
> }
>
> + if (fskb != skb_shinfo(skb)->frag_list)
> + goto perform_csum_check;
> +
> frag = skb_shinfo(nskb)->frags;
>
> skb_copy_from_linear_data_offset(skb, offset,
>
>