netdev - RE: [PATCH net v4] 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]

Date:	Tue, 3 Dec 2013 15:05:40 +0000
From:	Paul Durrant <Paul.Durrant@...rix.com>
To:	Wei Liu <wei.liu2@...rix.com>
CC:	Wei Liu <wei.liu2@...rix.com>,
	"xen-devel@...ts.xen.org" <xen-devel@...ts.xen.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Zoltan Kiss <zoltan.kiss@...rix.com>,
	Ian Campbell <Ian.Campbell@...rix.com>,
	David Vrabel <david.vrabel@...rix.com>,
	David Miller <davem@...emloft.net>
Subject: RE: [PATCH net v4] xen-netback: fix fragment detection in checksum
 setup

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@...rix.com]
> Sent: 03 December 2013 14:58
> To: Paul Durrant
> Cc: Wei Liu; xen-devel@...ts.xen.org; netdev@...r.kernel.org; Zoltan Kiss;
> Ian Campbell; David Vrabel; David Miller
> Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in checksum
> setup
> 
> On Tue, Dec 03, 2013 at 02:34:56PM +0000, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Wei Liu [mailto:wei.liu2@...rix.com]
> > > Sent: 03 December 2013 14:29
> > > To: Paul Durrant
> > > Cc: Wei Liu; xen-devel@...ts.xen.org; netdev@...r.kernel.org; Zoltan
> Kiss;
> > > Ian Campbell; David Vrabel; David Miller
> > > Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in
> checksum
> > > setup
> > >
> > > On Tue, Dec 03, 2013 at 02:05:17PM +0000, Paul Durrant wrote:
> > > [...]
> > > > > >
> > > > > > -	header_size = skb->network_header + off +
> MAX_IPOPTLEN;
> > > > > > -	maybe_pull_tail(skb, header_size);
> > > > > > +	if (!maybe_pull_tail(skb, sizeof(struct iphdr),
> MAX_IP_HDR_LEN))
> > > > > > +		goto out;
> > > > > > +
> > > > >
> > > > > I think you need to correctly update err to reflect this failure.
> > > > > Using -EPROTO will wrongly blame frontend while it is backend that's
> > > > > failing to process the packet.
> > > > >
> > > >
> > > > But a failure should only occur if the packet is malformed, so that would
> be
> > > a frontend error wouldn't it?
> > > >
> > >
> > > __pskb_pull_tail may fail due to malloc failure.
> > >
> > > However the return value of __pskb_pull_tail cannot reflect the wether
> > > the failure is due to malformed packet or OOM. Not sure what's the best
> > > solution here. What's the malformed packet you were talking about?
> > >
> >
> > For example, the pull would fail if the packet had an either_type of
> > IP but didn't contain an IP header, or perhaps an IPv6 packet that had
> > an incomplete option header sequence. I would have thought such a
> > packet was a more likely cause of failure than OOM, so -EPROTO seems a
> > reasonable best guess.
> 
> How? __pskb_pull_tail doesn't seem to care about upper layer protocols.
> And maybe_pull_tail has already done some lenght comparisions.
> 

No, __pskb_pull_tail() doesn't care but the final check in maybe_pull_tail() means it will return false if skb_headlen() is not at least as big as what it was asked for. So if we try to pull up an IP header and there's fewer bytes than that available then we hit the error condition. Or maybe I'm missing something.

  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