| Message ID | 1516334359.3606.36.camel@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] net: qdisc_pkt_len_init() should be more robust | expand |
On 2018年01月19日 11:59, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > Without proper validation of DODGY packets, we might very well > feed qdisc_pkt_len_init() with invalid GSO packets. > > tcp_hdrlen() might access out-of-bound data, so let's use > skb_header_pointer() and proper checks. > > Whole story is described in commit d0c081b49137 ("flow_dissector: > properly cap thoff field") > > We have the goal of validating DODGY packets earlier in the stack, > so we might very well revert this fix in the future. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Willem de Bruijn <willemb@google.com> > Cc: Jason Wang <jasowang@redhat.com> > Reported-by: syzbot+9da69ebac7dddd804552@syzkaller.appspotmail.com > --- > net/core/dev.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index 0e0ba36eeac9852b8df5ddd398dbc66ad6c83760..613fb4066be7bedbd3cd59ea9cf6eb7ef3100bd0 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -3151,10 +3151,21 @@ static void qdisc_pkt_len_init(struct sk_buff *skb) > hdr_len = skb_transport_header(skb) - skb_mac_header(skb); > > /* + transport layer */ > - if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) > - hdr_len += tcp_hdrlen(skb); > - else > - hdr_len += sizeof(struct udphdr); > + if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) { > + const struct tcphdr *th; > + struct tcphdr _tcphdr; > + > + th = skb_header_pointer(skb, skb_transport_offset(skb), > + sizeof(_tcphdr), &_tcphdr); > + if (likely(th)) > + hdr_len += __tcp_hdrlen(th); > + } else { > + struct udphdr _udphdr; > + > + if (skb_header_pointer(skb, skb_transport_offset(skb), > + sizeof(_udphdr), &_udphdr)) > + hdr_len += sizeof(struct udphdr); > + } > > if (shinfo->gso_type & SKB_GSO_DODGY) > gso_segs = DIV_ROUND_UP(skb->len - hdr_len, > Acked-by: Jason Wang <jasowang@redhat.com>
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Thu, 18 Jan 2018 19:59:19 -0800 > From: Eric Dumazet <edumazet@google.com> > > Without proper validation of DODGY packets, we might very well > feed qdisc_pkt_len_init() with invalid GSO packets. > > tcp_hdrlen() might access out-of-bound data, so let's use > skb_header_pointer() and proper checks. > > Whole story is described in commit d0c081b49137 ("flow_dissector: > properly cap thoff field") > > We have the goal of validating DODGY packets earlier in the stack, > so we might very well revert this fix in the future. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Willem de Bruijn <willemb@google.com> > Cc: Jason Wang <jasowang@redhat.com> > Reported-by: syzbot+9da69ebac7dddd804552@syzkaller.appspotmail.com Applied and queued up for -stable, thanks Eric.
diff --git a/net/core/dev.c b/net/core/dev.c index 0e0ba36eeac9852b8df5ddd398dbc66ad6c83760..613fb4066be7bedbd3cd59ea9cf6eb7ef3100bd0 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3151,10 +3151,21 @@ static void qdisc_pkt_len_init(struct sk_buff *skb) hdr_len = skb_transport_header(skb) - skb_mac_header(skb); /* + transport layer */ - if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) - hdr_len += tcp_hdrlen(skb); - else - hdr_len += sizeof(struct udphdr); + if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) { + const struct tcphdr *th; + struct tcphdr _tcphdr; + + th = skb_header_pointer(skb, skb_transport_offset(skb), + sizeof(_tcphdr), &_tcphdr); + if (likely(th)) + hdr_len += __tcp_hdrlen(th); + } else { + struct udphdr _udphdr; + + if (skb_header_pointer(skb, skb_transport_offset(skb), + sizeof(_udphdr), &_udphdr)) + hdr_len += sizeof(struct udphdr); + } if (shinfo->gso_type & SKB_GSO_DODGY) gso_segs = DIV_ROUND_UP(skb->len - hdr_len,