Re: [3/3] gso: Handle malicious GRO packets without crashing

Ben Hutchings
2013-11-07T18:18:39Z

On Thu, 2013-11-07 at 15:08 +0800, Herbert Xu wrote:
> As virtio_net can now generate GRO frag_list packets without
> sufficient verification, we need to handle malicious GRO packets
> thrown at us.
>
> This patch converts to affected BUG_ONs in skb_segment to rate-
> limited warnings.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index bcc3f1c..fb1106d 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2881,7 +2881,15 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> while (tail->next)
> tail = tail->next;
>
> - BUG_ON(fskb && tail->len != len + doffset);

Oh well, disregard my previous request for a comment.

> + if (fskb && tail->len != len + doffset) {
> + net_warn_ratelimited(
> + "skb_segment: "
> + "illegal GSO fragment: %u %u\n",
> + tail->len, len + doffset);
> + kfree(nskb);

kfree_skb()

> + err = -EINVAL;
> + goto err;
> + }
>
> len = nskb->len;
> kfree(nskb);
> @@ -2929,7 +2937,15 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> if (pos < offset + len) {
> struct sk_buff *fskb2 = fskb;
>
> - BUG_ON(pos + fskb->len != offset + len);
> + if (pos + fskb->len != offset + len) {
> + net_warn_ratelimited(
> + "skb_segment: "
> + "illegal GSO trailer: %u %u\n",
> + pos + fskb->len, offset + len);
> + kfree(nskb);

kfree_skb()

> + err = -EINVAL;
> + goto err;
> + }
>
> pos += fskb->len;
> fskb = fskb->next;

--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.