From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.toke.dk (mail.toke.dk [IPv6:2001:470:dc45:1000::1]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id AC7D33B29E for ; Fri, 27 Apr 2018 09:38:22 -0400 (EDT) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=20161023; t=1524836301; bh=rVxaJaLJveCLc440LowMpzQGDNQHIDrENQ66xZ+24Zg=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=r03NNAToy9Ge7FQXJvADoUgAt+oI6ZB5nAln7DcmRzzFgPoMqZh6rnK699iYfFWZa cW0QwjIrBaonZKwQchMvgJ7NtYYgOI74YY/sBG3zWEaSU7W2lf6dlp2S4GQ83zj0Dh G6u1YsbEt0xoc15TFnm0A4Qmt6eLrs8Dv8hamCsEqto2QoRm68l+SWeV0TI0YHzRDu XFjc1oV2lx20T3CU9IXBRJ6mSv4zl49N3Gbc0mRUwiOOkfwJ9jHve7izzazRNYGmQ9 m15en3BgRH9EKc4ws1UMzjWJShLeze5daRGwTLtOfrFFdA/Xdj34zMSf1CvmRxPUWb S9ag72S/Q1beA== To: Eric Dumazet , netdev@vger.kernel.org Cc: cake@lists.bufferbloat.net, Dave Taht In-Reply-To: References: <20180427121706.23273-1-toke@toke.dk> Date: Fri, 27 Apr 2018 15:38:20 +0200 X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <87in8c4vn7.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Cake] [PATCH net-next v4] Add Common Applications Kept Enhanced (cake) qdisc X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Apr 2018 13:38:22 -0000 Eric Dumazet writes: > On 04/27/2018 05:17 AM, Toke H=C3=B8iland-J=C3=B8rgensen wrote: > > ... > >> + >> +static struct sk_buff *cake_ack_filter(struct cake_sched_data *q, >> + struct cake_flow *flow) >> +{ >> + int seglen; >> + struct sk_buff *skb =3D flow->tail, *skb_check, *skb_check_prev; >> + struct iphdr *iph, *iph_check; >> + struct ipv6hdr *ipv6h, *ipv6h_check; >> + struct tcphdr *tcph, *tcph_check; >> + bool otherconn_ack_seen =3D false; >> + struct sk_buff *otherconn_checked_to =3D NULL; >> + bool thisconn_redundant_seen =3D false, thisconn_seen_last =3D false; >> + struct sk_buff *thisconn_checked_to =3D NULL, *thisconn_ack =3D NULL; >> + bool aggressive =3D q->ack_filter =3D=3D CAKE_ACK_AGGRESSIVE; >> + >> + /* no other possible ACKs to filter */ >> + if (flow->head =3D=3D skb) >> + return NULL; >> + >> + iph =3D skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb); >> + ipv6h =3D skb->encapsulation ? inner_ipv6_hdr(skb) : ipv6_hdr(skb); >> + >> + /* check that the innermost network header is v4/v6, and contains TCP = */ >> + if (pskb_may_pull(skb, ((unsigned char *)iph - skb->head) + sizeof(str= uct iphdr)) && >> + iph->version =3D=3D 4) { >> + if (iph->protocol !=3D IPPROTO_TCP) >> + return NULL; >> + seglen =3D ntohs(iph->tot_len) - (4 * iph->ihl); >> + tcph =3D (struct tcphdr *)((void *)iph + (4 * iph->ihl)); >> + if (!pskb_may_pull(skb, ((unsigned char *)tcph - skb->head) + sizeof(= struct tcphdr))) >> + return NULL; >> + } else if (pskb_may_pull(skb, ((unsigned char *)ipv6h - skb->head) + s= izeof(struct ipv6hdr) + sizeof(struct tcphdr)) && >> + ipv6h->version =3D=3D 6) { >> + if (ipv6h->nexthdr !=3D IPPROTO_TCP) >> + return NULL; >> + seglen =3D ntohs(ipv6h->payload_len); >> + tcph =3D (struct tcphdr *)((void *)ipv6h + >> + sizeof(struct ipv6hdr)); >> + } else { >> + return NULL; >> + } >> + > > > This is still broken. > > After pskb_may_pull(), skb->head might have been reallocated. > > You need to recompute iph , ipv6h, tcph, otherwise you are reading > freed memory and crash kernels with sufficient debugging (KASAN and > other CONFIG_DEBUG_PAGEALLOC / CONFIG_DEBUG_SLAB like options) Ah, right. Will fix. Is it safe to dereference the iph pointer before calling pskb_may_pull()? -Toke