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 for Android: free password hash cracker in your pocket

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

Message-ID: <20130501130107.GH9780@zion.uk.xensource.com>
Date:	Wed, 1 May 2013 14:01:07 +0100
From:	Wei Liu <wei.liu2@...rix.com>
To:	Ian Campbell <Ian.Campbell@...rix.com>
CC:	Wei Liu <wei.liu2@...rix.com>,
	"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, May 01, 2013 at 12:47:06PM +0100, Ian Campbell wrote:
> On Wed, 2013-05-01 at 12:40 +0100, Wei Liu wrote:
> > On Wed, May 01, 2013 at 12:21:43PM +0100, Ian Campbell wrote:
> > > 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.
> > > 
> > 
> > Any packet using more than XEN_NETIF_NR_SLOTS_MIN are considered
> > malformed at this point. The behavior is documented in previous commit
> > log. 2810e5b9a "xen-netback: coalesce slots in TX path and fix
> > regressions".
> > 
> > """
> > The behavior of netback for packet is thus:
> > 
> >         1-18            slots: valid
> >        19-max_skb_slots slots: drop and respond with an error
> >        max_skb_slots+   slots: fatal error
> > """
> 
> OK, so my understanding was wrong and this patch is doing the right
> thing.
> 
> However it does seem rather like NR_SLOTS_MIN and max_skb_slots are a
> bit misnamed. They are actually NR_SLOTS_MAX and fatal_skb_slots? The
> NR_SLOTS{MIN/MAX} disparity is particularly confusing in the context of
> this code (I understand its the minimum that a backend must support, but
> its still confusing in the context of these functions).
> 

Yes probably the naming is weird.

Probably we can do

  #define XEN_NETBK_SLOTS_MAX XEN_NETIF_NR_SLOTS_MIN
  max_skb_slots -> fatal_skb_slots

if it makes things clearer.


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