netdev - Re: [PATCH] CHOKe flow scheduler (0.9)

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: <1295379257.9097.9.camel@edumazet-laptop>
Date:	Tue, 18 Jan 2011 20:34:17 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Stephen Hemminger <shemminger@...tta.com>
Cc:	Patrick McHardy <kaber@...sh.net>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] CHOKe flow scheduler (0.9)

Le mardi 18 janvier 2011 à 11:06 -0800, Stephen Hemminger a écrit :

> +static bool choke_match_flow(struct sk_buff *skb1, struct sk_buff *skb2)
> +{
> +	int off1, off2, poff;
> +	u8 ip_proto;
> +	u32 ihl;
> +
> +	if (skb1->protocol != skb2->protocol)
> +		return false;
> +
> +	off1 = skb_network_offset(skb1);
> +	off2 = skb_network_offset(skb2);

> +
> +	switch (skb1->protocol) {
> +	case __constant_htons(ETH_P_IP): {
> +		struct iphdr *ip1, *ip2;
> +
> +		if (!pskb_may_pull(skb1, sizeof(struct iphdr) + off1))
> +			return false;
> +
	pskb_network_may_pull() might be cleaner


> +		ip1 = (struct iphdr *) (skb1->data + off1);
	ip1 = ip_hdr(skb);

> +		if (ip1->frag_off & htons(IP_MF | IP_OFFSET))
> +			return false;	/* don't compare fragments */
> +

Hmm, we should compare fragments if possible.

saddr/daddr are available, not the ports.

> +		if (!pskb_may_pull(skb2, sizeof(struct iphdr) + off2))
> +			return false;
> +
> +		ip2 = (struct iphdr *) (skb2->data + off2);
> +		if (ip2->frag_off & htons(IP_MF | IP_OFFSET))
> +			return false;
> +
> +		if (ip1->protocol != ip2->protocol ||
> +		    ip1->saddr != ip2->saddr || ip1->daddr != ip2->daddr)
> +			return false;
> +


	What happens if ip1->ihl != ip2->ihl  here ?

Here I would add the fragment test :

	if ((ip1->frag_off | ip2->frag_off)) & htons(IP_MF | IP_OFFSET))
		return true;

> +		ip_proto = ip1->protocol;
> +		ihl = ip1->ihl;
> +		break;
> +	}
> +
> +	case __constant_htons(ETH_P_IPV6): {
> +		struct ipv6hdr *ip1, *ip2;
> +
> +		if (!pskb_may_pull(skb1, sizeof(struct ipv6hdr *) + off1))
> +			return false;

ouch... sizeof(sizeof(struct ipv6hdr *) is not what you want but
sizeof(struct ipv6hdr) is.

So just use :

	pskb_network_may_pull(skb1, sizeof(*ip1))
> +
> +		if (!pskb_may_pull(skb2, sizeof(struct ipv6hdr *) + off2))
> +			return false;
> +
> +		ip1 = (struct ipv6hdr *) (skb1->data + off1);
	ip1 = ipv6_hdr(skb1);
> +		ip2 = (struct ipv6hdr *) (skb2->data + off2);
> +
> +		if (ip1->nexthdr != ip2->nexthdr ||
> +		    ipv6_addr_cmp(&ip1->saddr, &ip2->saddr) != 0 ||
> +		    ipv6_addr_cmp(&ip1->daddr, &ip2->daddr))
> +			return false;
> +
> +		ihl = (40 >> 2);
> +		ip_proto = ip1->nexthdr;
> +		break;
> +	}
> +
> +	default:
> +		return false;
> +	}
> +



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html