netdev - Re: [PATCH net-next V2 2/3] 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: <1367510146.21869.136.camel@zakaz.uk.xensource.com>
Date:	Thu, 2 May 2013 16:55:46 +0100
From:	Ian Campbell <Ian.Campbell@...rix.com>
To:	Jan Beulich <JBeulich@...e.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>
Subject: Re: [PATCH net-next V2 2/3] xen-netback: avoid allocating variable
 size array on stack

On Thu, 2013-05-02 at 13:04 +0100, Jan Beulich wrote:
> >>> On 02.05.13 at 12:43, Wei Liu <wei.liu2@...rix.com> wrote:
> > @@ -934,11 +934,14 @@ static int netbk_count_requests(struct xenvif *vif,
> >  	RING_IDX cons = vif->tx.req_cons;
> >  	int slots = 0;
> >  	int drop_err = 0;
> > +	int more_data;
> >  
> >  	if (!(first->flags & XEN_NETTXF_more_data))
> >  		return 0;
> >  
> >  	do {
> > +		struct xen_netif_tx_request dropped_tx = { 0 };
> > +
> 
> No need for an initializer here.
> 
> >  		if (slots >= work_to_do) {
> >  			netdev_err(vif->dev,
> >  				   "Asked for %d slots but exceeds this limit\n",
> > @@ -972,6 +975,9 @@ static int netbk_count_requests(struct xenvif *vif,
> >  			drop_err = -E2BIG;
> >  		}
> >  
> > +		if (drop_err)
> > +			txp = &dropped_tx;
> > +
> >  		memcpy(txp, RING_GET_REQUEST(&vif->tx, cons + slots),
> >  		       sizeof(*txp));
> >  
> > @@ -1001,7 +1007,13 @@ static int netbk_count_requests(struct xenvif *vif,
> >  			netbk_fatal_tx_err(vif);
> >  			return -EINVAL;
> >  		}
> > -	} while ((txp++)->flags & XEN_NETTXF_more_data);
> > +
> > +		more_data = txp->flags & XEN_NETTXF_more_data;
> > +
> > +		if (!drop_err)
> > +			txp++;
> 
> And no need for the conditional here afaict.

I think it is needed, in the case where you've assigned txp =
&dropped_tx you don't want to increment txp.

Or perhaps it just gets reassigned back to &dropped_tx at the top of the
next loop, so the increment doesn't matter. Subtle! I'm happy with
whichever way Wei prefers, but it is probably worthy of a comment

> 
> Jan
> 
> > +
> > +	} while (more_data);
> >  
> >  	if (drop_err) {
> >  		netbk_tx_err(vif, first, cons + slots);
> 
> 


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