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
