From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH] xen-netfront: drop skb when skb->len > 65535 Date: Sat, 2 Mar 2013 13:32:44 +0000 Message-ID: <20130302133243.GA6846@zion.uk.xensource.com> References: <1362155488-24316-1-git-send-email-wei.liu2@citrix.com> <5130E9D602000078000C27CC@nat28.tlf.novell.com> <1362157246.2109.165.camel@zion.uk.xensource.com> <1362192857.4198.9.camel@hastur.hellion.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1362192857.4198.9.camel@hastur.hellion.org.uk> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: "ij@2013.bluespice.org" , konrad.wilk@oracle.com, "npegg@linode.com" , "xen-devel@lists.xen.org" , "annie.li@oracle.com" , Jan Beulich List-Id: xen-devel@lists.xenproject.org On Sat, Mar 02, 2013 at 02:54:17AM +0000, Ian Campbell wrote: > On Fri, 2013-03-01 at 17:00 +0000, Wei Liu wrote: > > On Fri, 2013-03-01 at 16:48 +0000, Jan Beulich wrote: > > > >>> On 01.03.13 at 17:31, Wei Liu wrote: > > > > The `size' field of Xen network wired format is uint16_t, anything bigger > > > > than > > > > 65535 will cause overflow. > > > > > > > > The punishment introduced by XSA-39 is quite harsh - DomU is disconnected when > > > > it's discovered to be sending corrupted skbs. However, it looks like Linux > > > > kernel will generate some bad skbs sometimes, so drop those skbs before > > > > sending to over netback to avoid being disconnected. > > > > > > While fixing the frontend is certainly desirable, we can't expect > > > everyone to deploy fixed netfronts in all their VMs - some OS > > > versions used in there may even be out of service. So we > > > ought to find a way to also more gracefully deal with the > > > situation in netback, without re-opening the security issue > > > that prompted those changes. > > > > > > > Regarding the punishment bit, I think its worth discussing it a bit. > > Yes, the trick is figuring out what to do without reintroducing the > softlockup which XSA-39 fixed. > > Perhaps we should allow silently consume (and drop) oversize skbs and > only shutdown the rings if they also consume too many (FSVO too many) > slots? > > > But the bug is always there, it drew no attention until revealed by > > XSA-39. It ought to be fixed anyway. :-) > > I would have sworn that skb->len was also limited to 64k, but looking at > the header I see it is actually an int and the only limit of that sort > is related to MAX_SKB_FRAGS (which doesn't actually limit the total > size). I had the impression that skb->len was limited to 64k, too. But it turned out I was wrong. > > OOI how big were the skbs you were seeing? As Nick (npegg@linode.com) pointed out in his email, he saw size 65538. I can reproduce this as well by setting vif's mtu to 100 then run iperf. 100 was just a random number I came up with when I played with fragmentation. > > Not that it really matters but do we have a handle on why the prexisting > bug didn't already cause connectivity issues? Does the retransmit (which > I suppose must be happening) somehow end up using a smaller skb size? > Not sure. I didn't have enough time to look into this yesterday. :-( > BTW you mean "wire protocol" not "wired protocol" in the comments etc. > Yes. Wei. > Ian. >