Eric Dumazet <eric.duma...@gmail.com> writes: > On 04/27/2018 05:17 AM, Toke Høiland-Jørgensen wrote: > > ... > >> + >> +static struct sk_buff *cake_ack_filter(struct cake_sched_data *q, >> + struct cake_flow *flow) >> +{ >> + int seglen; >> + struct sk_buff *skb = 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 = false; >> + struct sk_buff *otherconn_checked_to = NULL; >> + bool thisconn_redundant_seen = false, thisconn_seen_last = false; >> + struct sk_buff *thisconn_checked_to = NULL, *thisconn_ack = NULL; >> + bool aggressive = q->ack_filter == CAKE_ACK_AGGRESSIVE; >> + >> + /* no other possible ACKs to filter */ >> + if (flow->head == skb) >> + return NULL; >> + >> + iph = skb->encapsulation ? inner_ip_hdr(skb) : ip_hdr(skb); >> + ipv6h = 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(struct iphdr)) && >> + iph->version == 4) { >> + if (iph->protocol != IPPROTO_TCP) >> + return NULL; >> + seglen = ntohs(iph->tot_len) - (4 * iph->ihl); >> + tcph = (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) + >> sizeof(struct ipv6hdr) + sizeof(struct tcphdr)) && >> + ipv6h->version == 6) { >> + if (ipv6h->nexthdr != IPPROTO_TCP) >> + return NULL; >> + seglen = ntohs(ipv6h->payload_len); >> + tcph = (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 _______________________________________________ Cake mailing list Cake@lists.bufferbloat.net https://lists.bufferbloat.net/listinfo/cake