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

[Andy Whitcroft]

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.

-apw