From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [PATCH net] net: qdisc_pkt_len_init() should be more robust Date: Fri, 19 Jan 2018 20:40:17 +0800 Message-ID: <9ea9d83a-56b5-a7a9-c0c1-0d37f5d87260@redhat.com> References: <1516334359.3606.36.camel@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: netdev , Willem de Bruijn To: Eric Dumazet , David Miller Return-path: Received: from mx1.redhat.com ([209.132.183.28]:45402 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755621AbeASMkW (ORCPT ); Fri, 19 Jan 2018 07:40:22 -0500 In-Reply-To: <1516334359.3606.36.camel@gmail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 2018年01月19日 11:59, Eric Dumazet wrote: > From: Eric Dumazet > > 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 > Cc: Willem de Bruijn > Cc: Jason Wang > 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