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 for Android: free password hash cracker in your pocket

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

Message-ID: <9AAE0902D5BC7E449B7C8E4E778ABCD01ACF1F@AMSPEX01CL01.citrite.net>
Date:	Tue, 10 Dec 2013 16:24:12 +0000
From:	Paul Durrant <Paul.Durrant@...rix.com>
To:	Jan Beulich <JBeulich@...e.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" <netdev@...r.kernel.org>
Subject: RE: [Xen-devel] [PATCH net v5] xen-netback: fix fragment detection
 in checksum setup

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@...e.com]
> Sent: 10 December 2013 16:12
> To: Paul Durrant
> Cc: David Vrabel; Ian Campbell; Wei Liu; Zoltan Kiss; David Miller; xen-devel;
> 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.
> 

Yes, I guess that's true but if we decide to pull up at all then is it harmful to pull more than we absolutely need?

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