netdev - Re: [PATCH net-next v4] Add Common Applications Kept Enhanced (cake) qdisc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <efb401ac-bc79-cbc5-cd03-120803b65b4d@gmail.com>
Date:   Fri, 27 Apr 2018 06:17:17 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Toke Høiland-Jørgensen <toke@...e.dk>,
        netdev@...r.kernel.org
Cc:     cake@...ts.bufferbloat.net, Dave Taht <dave.taht@...il.com>
Subject: Re: [PATCH net-next v4] Add Common Applications Kept Enhanced (cake)
 qdisc



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)