From mboxrd@z Thu Jan 1 00:00:00 1970 From: annie li Subject: Re: [PATCH] xen-netfront: drop skb when skb->len > 65535 Date: Sun, 03 Mar 2013 12:13:42 +0800 Message-ID: <5132CDF6.7020700@oracle.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"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: 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@citrix.com" , "npegg@linode.com" , "xen-devel@lists.xen.org" , Jan Beulich , Wei Liu List-Id: xen-devel@lists.xenproject.org On 2013-3-2 10:54, 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). If compound page enabled, it is very possible that netback runs into following code. if (unlikely(frags >= MAX_SKB_FRAGS)) { netdev_err(vif->dev, "Too many frags\n"); netbk_fatal_tx_err(vif); return -frags; } MAX_SKB_FRAGS does not actually limit the total size in theory. But it does limit the total size in netback since netback only supports one-page-sized fragments. Thanks Annie