netdev - Re: [PATCH net-next 2/2] xen-netback: avoid allocating variable size array on stack

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: <1367407303.3142.694.camel@zakaz.uk.xensource.com>
Date:	Wed, 1 May 2013 12:21:43 +0100
From:	Ian Campbell <Ian.Campbell@...rix.com>
To:	Wei Liu <wei.liu2@...rix.com>
CC:	"xen-devel@...ts.xen.org" <xen-devel@...ts.xen.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"jbeulich@...e.com" <jbeulich@...e.com>
Subject: Re: [PATCH net-next 2/2] xen-netback: avoid allocating variable
 size array on stack

On Wed, 2013-05-01 at 11:53 +0100, Wei Liu wrote:
> On Wed, May 01, 2013 at 11:32:41AM +0100, Ian Campbell wrote:
> > On Tue, 2013-04-30 at 17:50 +0100, Wei Liu wrote:
> > > Tune xen_netbk_count_requests to not touch working array beyond limit, so that
> > > we can make working array size constant.
> > 
> > Is this really correct when max_skb_slots > XEN_NETIF_NR_SLOTS_MIN?
> > Seems like we would either overrun the array or drop frames which
> > max_skb_slots suggests we should accept?
> > 
> 
> So the max_skb_slots for now is the standard to determine whether a
> guest is malicious, not the maximum slots we can process.

Perhaps I've have misunderstood this patch then but it looks to me like
it will cause us to drop skbs which use slots > XEN_NETIF_NR_SLOTS_MIN
and < max_skb_slots, i.e. ones which are considered non-malicious by the
above definition. Or it will cause us to access indexes into
xen_netbk_tx_build_gops.txfrags which are > XEN_NETIF_NR_SLOTS_MIN.

If XEN_NETIF_NR_SLOTS_MIN==18 and max_skb_slots == 22 what will this
patch cause to happen to an SKB which uses 20 slots? Will it be dropped
or will it access index 20 into an array with size 18?

> > Other options:
> > 
> > Handle batches of work in <max_skb_slots sized bundles, but that gets
> > complex when you consider the case of an skb which crosses multiple such
> > bundles.
> > 
> > xen_netbk_get_requests() copes the tx req again into the pending_tx_info
> > -- any way we can arrange for this to just happen right in the first
> > place?
> > 
> 
> Isn't the point of having xen_netbk_count_requests to drop malformed
> packets before wasting any effort processing them?

Yes, but it seems to me like you are dropping non-malformed packets.

Also remember that the tx requests accumulated by
xen_netbk_count_requests into the txfrags array are subsequently used by
xen_netbk_get_requests to do the actual processing.

Ian.

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