Re: [Xen-devel] [PATCH] xen-netfront: drop skb when skb->len > 65535

[annie li <annie.li@xxxxxxxxxx>]

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 <wei.liu2@xxxxxxxxxx> 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel