[Acked] [CVE-2014-0131 3/3] skbuff: skb_segment: orphan frags before copying

[Andy Whitcroft]

On Fri, Jun 13, 2014 at 03:58:35PM +0100, Andy Whitcroft wrote:
> On Fri, Jun 13, 2014 at 12:12:28PM +0100, Luis Henriques wrote:
> > From: "Michael S. Tsirkin" <mst at redhat.com>
> > 
> > skb_segment copies frags around, so we need
> > to copy them carefully to avoid accessing
> > user memory after reporting completion to userspace
> > through a callback.
> > 
> > skb_segment doesn't normally happen on datapath:
> > TSO needs to be disabled - so disabling zero copy
> > in this case does not look like a big deal.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> > Acked-by: Herbert Xu <herbert at gondor.apana.org.au>
> > Signed-off-by: David S. Miller <davem at davemloft.net>
> > (back ported from commit 1fd819ecb90cc9b822cd84d3056ddba315d3340f)
> > CVE-2014-0131
> > BugLink: http://bugs.launchpad.net/bugs/1298119
> > Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
> > ---
> >  net/core/skbuff.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index 66e03b1dec72..99a65b4e4adc 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -2739,6 +2739,9 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> >  						 skb_put(nskb, hsize), hsize);
> >  
> >  		while (pos < offset + len && i < nfrags) {
> > +			if (unlikely(skb_orphan_frags(skb, GFP_ATOMIC)))
> > +				goto err;
> > +
> >  			*frag = skb_shinfo(skb)->frags[i];
> >  			__skb_frag_ref(frag);
> >  			size = skb_frag_size(frag);
> > -- 
> > 1.9.1
> 
> Ok, this seems viable to my eye, but only under the constraints of the
> upstream discussion.  I think we need to check with Ben if the feedback
> and testing he was asking for ever happened as Dave Miller seemed a bit
> hesitant too.
> 
> Failing that I think we want to figure out if we can test this.

Ok  this discussion already occured, and this patch got applied across
the other stables, based on that.

Acked-by: Andy Whitcroft <apw at canonical.com>

-apw