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
| ||
|
Date: Tue, 3 Dec 2013 15:13:29 +0000 From: Wei Liu <wei.liu2@...rix.com> To: Paul Durrant <Paul.Durrant@...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 On Tue, Dec 03, 2013 at 03:05:40PM +0000, Paul Durrant wrote: > > -----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. OK, we'er still on the same boat here. ;-) Would it make sense to make maybe_pull_tail to return int to reflect __pskb_pull_fail? In that case we can distinguish backend failure and frontend failure. I pay extra attention to this as we often have no access to frontend and we probably don't want to blame frontend for non-existent misbehavior. Wei. > > 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
Powered by blists - more mailing lists