| Message ID | 20131107070847.GC31638@gondor.apana.org.au |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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;
Hello. On 11/07/2013 10:08 AM, 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); > + if (fskb && tail->len != len + doffset) { > + net_warn_ratelimited( > + "skb_segment: " > + "illegal GSO fragment: %u %u\n", Don't break up the message -- chekpatch.pl should allow that... > @@ -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", Same here. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 07, 2013 at 10:13:29PM +0300, Sergei Shtylyov wrote: > Hello. > > On 11/07/2013 10:08 AM, 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); > >+ if (fskb && tail->len != len + doffset) { > >+ net_warn_ratelimited( > >+ "skb_segment: " > >+ "illegal GSO fragment: %u %u\n", > > Don't break up the message -- chekpatch.pl should allow that... Thanks for the comment. In the latest version of this patch this should no longer be an issue.
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); + if (fskb && tail->len != len + doffset) { + net_warn_ratelimited( + "skb_segment: " + "illegal GSO fragment: %u %u\n", + tail->len, len + doffset); + kfree(nskb); + 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); + err = -EINVAL; + goto err; + } pos += fskb->len; fskb = fskb->next;
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>