netdev - Re: [Xen-devel] [PATCH net v5] xen-netback: fix fragment detection in checksum setup

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: <52A74B56020000780010BFB9@nat28.tlf.novell.com>
Date:	Tue, 10 Dec 2013 16:11:50 +0000
From:	"Jan Beulich" <JBeulich@...e.com>
To:	"Paul Durrant" <paul.durrant@...rix.com>
Cc:	"David Vrabel" <david.vrabel@...rix.com>,
	"Ian Campbell" <ian.campbell@...rix.com>,
	"Wei Liu" <wei.liu2@...rix.com>,
	"Zoltan Kiss" <zoltan.kiss@...rix.com>,
	"David Miller" <davem@...emloft.net>,
	"xen-devel" <xen-devel@...ts.xenproject.org>,
	<netdev@...r.kernel.org>
Subject: Re: [Xen-devel] [PATCH net v5] xen-netback: fix fragment
 detection in checksum setup

>>> On 03.12.13 at 18:39, Paul Durrant <paul.durrant@...rix.com> wrote:
>  static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
>  			     int recalculate_partial_csum)
>  {
> -	struct iphdr *iph = (void *)skb->data;
> -	unsigned int header_size;
>  	unsigned int off;
> -	int err = -EPROTO;
> +	bool fragment;
> +	int err;
> +
> +	fragment = false;
> +
> +	err = maybe_pull_tail(skb,
> +			      sizeof(struct iphdr),
> +			      MAX_IP_HDR_LEN);
> +	if (err < 0)
> +		goto out;
>  
> -	off = sizeof(struct iphdr);
> +	if (ip_hdr(skb)->frag_off & htons(IP_OFFSET | IP_MF))
> +		fragment = true;

You don't seem to be using "fragment" anywhere.

>  
> -	header_size = skb->network_header + off + MAX_IPOPTLEN;
> -	maybe_pull_tail(skb, header_size);
> +	off = ip_hdrlen(skb);
>  
> -	off = iph->ihl * 4;
> +	err = -EPROTO;
>  
> -	switch (iph->protocol) {
> +	switch (ip_hdr(skb)->protocol) {
>  	case IPPROTO_TCP:
>  		if (!skb_partial_csum_set(skb, off,
>  					  offsetof(struct tcphdr, check)))
>  			goto out;
>  
>  		if (recalculate_partial_csum) {
> -			struct tcphdr *tcph = tcp_hdr(skb);
> -
> -			header_size = skb->network_header +
> -				off +
> -				sizeof(struct tcphdr);
> -			maybe_pull_tail(skb, header_size);
> -
> -			tcph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
> -							 skb->len - off,
> -							 IPPROTO_TCP, 0);
> +			err = maybe_pull_tail(skb,
> +					      off + sizeof(struct tcphdr),
> +					      MAX_IP_HDR_LEN);

Is it really necessary/worthwhile to specify MAX_IP_HDR_LEN
here? Other than in the IPv6 case you're not risking to need
another pull if you simply used off + sizeof(struct tcphdr) instead.

Jan

--
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