netdev - Re: [PATCH V5 6/7] xen-netback: coalesce slots in TX path and fix regressions

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130422092825.GN5815@zion.uk.xensource.com>
Date:	Mon, 22 Apr 2013 10:28:25 +0100
From:	Wei Liu <wei.liu2@...rix.com>
To:	annie li <annie.li@...cle.com>
CC:	Wei Liu <wei.liu2@...rix.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"xen-devel@...ts.xen.org" <xen-devel@...ts.xen.org>,
	"konrad.wilk@...cle.com" <konrad.wilk@...cle.com>,
	"jbeulich@...e.com" <jbeulich@...e.com>,
	Ian Campbell <Ian.Campbell@...rix.com>,
	"wdauchy@...il.com" <wdauchy@...il.com>,
	David Vrabel <david.vrabel@...rix.com>
Subject: Re: [PATCH V5 6/7] xen-netback: coalesce slots in TX path and fix
 regressions

On Sun, Apr 21, 2013 at 11:06:10PM +0100, annie li wrote:
[...]
> > -		if (unlikely(frags >= MAX_SKB_FRAGS)) {
> > -			netdev_err(vif->dev, "Too many frags\n");
> > +		/* This guest is really using too many slots and
> > +		 * considered malicious.
> > +		 */
> > +		if (unlikely(slots >= max_skb_slots)) {
> > +			netdev_err(vif->dev,
> > +				   "Malicious frontend using %d slots, threshold %u\n",
> > +				   slots, max_skb_slots);
> >   			netbk_fatal_tx_err(vif);
> >   			return -E2BIG;
> 
> It is possible that vif is freed when packet size is less than 64K here 
> but slots required >= max_skb_slots. Alough max_skb_slots can be 
> configured, this kind of packets would be dropped in following if 
> (!drop_err && slots >= XEN_NETIF_NR_SLOTS_MIN).
> 

Yes, because slots required >= max_skb_slots is considered malicious
behavior. The size of the packet doesn't matter here.

> >   		}
> >   
> > -		memcpy(txp, RING_GET_REQUEST(&vif->tx, cons + frags),
> > +		/* Xen network protocol had implicit dependency on
> > +		 * MAX_SKB_FRAGS. XEN_NETIF_NR_SLOTS_MIN is set to the
> > +		 * historical MAX_SKB_FRAGS value 18 to honor the same
> > +		 * behavior as before. Any packet using more than 18
> > +		 * slots but less than max_skb_slots slots is dropped
> > +		 */
> > +		if (!drop_err && slots >= XEN_NETIF_NR_SLOTS_MIN) {
> > +			if (net_ratelimit())
> > +				netdev_dbg(vif->dev,
> > +					   "Too many slots (%d) exceeding limit (%d), dropping packet\n",
> > +					   slots, XEN_NETIF_NR_SLOTS_MIN);
> > +			drop_err = -E2BIG;
> 
> It is possible to drop packets like above(size < 64K && slot >= 
> XEN_NETIF_NR_SLOTS_MIN). I do not know how frequently this kind of 
> packets appear, maybe some SKBs with compound page(size < 64K && slot >= 
> XEN_NETIF_NR_SLOTS_MIN) are dropped here?
> 

I don't know either, but this is the default behavior of netback so here
we retain the same behavior here to avoid further regression.


Wei.
--
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