Fix CVE-2012-2119 and other critical bugs in macvtap zerocopy

svn path=/dists/sid/linux/; revision=19121

7 files changed, 275 insertions, 0 deletions

diff --git a/debian/changelog b/debian/changelog
index 1897afeb2e0f..6de4d61f4505 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -51,6 +51,12 @@ linux (3.2.20-1) UNRELEASED; urgency=low
   * media/dvb: Enable DVB_DDBRIDGE as module (Closes: #676952)
   * net: sock: validate data_len before allocating skb in
     sock_alloc_send_pskb() (CVE-2012-2136)
+  * macvtap: zerocopy: fix offset calculation when building skb
+  * macvtap: zerocopy: fix truesize underestimation
+  * macvtap: zerocopy: put page when fail to get all requested user pages
+  * macvtap: zerocopy: set SKBTX_DEV_ZEROCOPY only when skb is built
+    successfully
+  * macvtap: zerocopy: validate vectors before building skb (CVE-2012-2119)
 
   [ Bastian Blank ]
   * [s390/s390x,s390x/s390x] Build debugging symbols.
diff --git a/debian/patches/bugfix/all/macvtap-zerocopy-fix-offset-calculation-when-buildin.patch b/debian/patches/bugfix/all/macvtap-zerocopy-fix-offset-calculation-when-buildin.patch
new file mode 100644
index 000000000000..9110768142a4
--- /dev/null
+++ b/debian/patches/bugfix/all/macvtap-zerocopy-fix-offset-calculation-when-buildin.patch
@@ -0,0 +1,61 @@
+From: Jason Wang <jasowang@redhat.com>
+Date: Wed, 2 May 2012 11:41:30 +0800
+Subject: [1/5] macvtap: zerocopy: fix offset calculation when building skb
+
+commit 3afc9621f15701c557e60f61eba9242bac2771dd upstream.
+
+This patch fixes the offset calculation when building skb:
+
+- offset1 were used as skb data offset not vector offset
+- reset offset to zero only when we advance to next vector
+
+Signed-off-by: Jason Wang <jasowang@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+---
+ drivers/net/macvtap.c |   13 +++++++------
+ 1 file changed, 7 insertions(+), 6 deletions(-)
+
+diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
+index 0427c65..bd4a70d 100644
+--- a/drivers/net/macvtap.c
++++ b/drivers/net/macvtap.c
+@@ -505,10 +505,11 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
+ 		if (copy > size) {
+ 			++from;
+ 			--count;
+-		}
++			offset = 0;
++		} else
++			offset += size;
+ 		copy -= size;
+ 		offset1 += size;
+-		offset = 0;
+ 	}
+ 
+ 	if (len == offset1)
+@@ -519,13 +520,13 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
+ 		int num_pages;
+ 		unsigned long base;
+ 
+-		len = from->iov_len - offset1;
++		len = from->iov_len - offset;
+ 		if (!len) {
+-			offset1 = 0;
++			offset = 0;
+ 			++from;
+ 			continue;
+ 		}
+-		base = (unsigned long)from->iov_base + offset1;
++		base = (unsigned long)from->iov_base + offset;
+ 		size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
+ 		num_pages = get_user_pages_fast(base, size, 0, &page[i]);
+ 		if ((num_pages != size) ||
+@@ -546,7 +547,7 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
+ 			len -= size;
+ 			i++;
+ 		}
+-		offset1 = 0;
++		offset = 0;
+ 		++from;
+ 	}
+ 	return 0;
diff --git a/debian/patches/bugfix/all/macvtap-zerocopy-fix-truesize-underestimation.patch b/debian/patches/bugfix/all/macvtap-zerocopy-fix-truesize-underestimation.patch
new file mode 100644
index 000000000000..75164f0a2b5c
--- /dev/null
+++ b/debian/patches/bugfix/all/macvtap-zerocopy-fix-truesize-underestimation.patch
@@ -0,0 +1,41 @@
+From: Jason Wang <jasowang@redhat.com>
+Date: Wed, 2 May 2012 11:41:44 +0800
+Subject: [2/5] macvtap: zerocopy: fix truesize underestimation
+
+commit 4ef67ebedffa44ed9939b34708ac2fee06d2f65f upstream.
+
+As the skb fragment were pinned/built from user pages, we should
+account the page instead of length for truesize.
+
+Signed-off-by: Jason Wang <jasowang@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+---
+ drivers/net/macvtap.c |    6 ++++--
+ 1 file changed, 4 insertions(+), 2 deletions(-)
+
+diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
+index bd4a70d..7cb2684 100644
+--- a/drivers/net/macvtap.c
++++ b/drivers/net/macvtap.c
+@@ -519,6 +519,7 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
+ 		struct page *page[MAX_SKB_FRAGS];
+ 		int num_pages;
+ 		unsigned long base;
++		unsigned long truesize;
+ 
+ 		len = from->iov_len - offset;
+ 		if (!len) {
+@@ -533,10 +534,11 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
+ 		    (num_pages > MAX_SKB_FRAGS - skb_shinfo(skb)->nr_frags))
+ 			/* put_page is in skb free */
+ 			return -EFAULT;
++		truesize = size * PAGE_SIZE;
+ 		skb->data_len += len;
+ 		skb->len += len;
+-		skb->truesize += len;
+-		atomic_add(len, &skb->sk->sk_wmem_alloc);
++		skb->truesize += truesize;
++		atomic_add(truesize, &skb->sk->sk_wmem_alloc);
+ 		while (len) {
+ 			int off = base & ~PAGE_MASK;
+ 			int size = min_t(int, len, PAGE_SIZE - off);
diff --git a/debian/patches/bugfix/all/macvtap-zerocopy-put-page-when-fail-to-get-all-reque.patch b/debian/patches/bugfix/all/macvtap-zerocopy-put-page-when-fail-to-get-all-reque.patch
new file mode 100644
index 000000000000..1674eb84feb6
--- /dev/null
+++ b/debian/patches/bugfix/all/macvtap-zerocopy-put-page-when-fail-to-get-all-reque.patch
@@ -0,0 +1,35 @@
+From: Jason Wang <jasowang@redhat.com>
+Date: Wed, 2 May 2012 11:41:58 +0800
+Subject: [3/5] macvtap: zerocopy: put page when fail to get all requested
+ user pages
+
+commit 02ce04bb3d28c3333231f43bca677228dbc686fe upstream.
+
+When get_user_pages_fast() fails to get all requested pages, we could not use
+kfree_skb() to free it as it has not been put in the skb fragments. So we need
+to call put_page() instead.
+
+Signed-off-by: Jason Wang <jasowang@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+---
+ drivers/net/macvtap.c |    6 ++++--
+ 1 file changed, 4 insertions(+), 2 deletions(-)
+
+diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
+index 7cb2684..9ab182a 100644
+--- a/drivers/net/macvtap.c
++++ b/drivers/net/macvtap.c
+@@ -531,9 +531,11 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
+ 		size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
+ 		num_pages = get_user_pages_fast(base, size, 0, &page[i]);
+ 		if ((num_pages != size) ||
+-		    (num_pages > MAX_SKB_FRAGS - skb_shinfo(skb)->nr_frags))
+-			/* put_page is in skb free */
++		    (num_pages > MAX_SKB_FRAGS - skb_shinfo(skb)->nr_frags)) {
++			for (i = 0; i < num_pages; i++)
++				put_page(page[i]);
+ 			return -EFAULT;
++		}
+ 		truesize = size * PAGE_SIZE;
+ 		skb->data_len += len;
+ 		skb->len += len;
diff --git a/debian/patches/bugfix/all/macvtap-zerocopy-set-SKBTX_DEV_ZEROCOPY-only-when-sk.patch b/debian/patches/bugfix/all/macvtap-zerocopy-set-SKBTX_DEV_ZEROCOPY-only-when-sk.patch
new file mode 100644
index 000000000000..fb5c202565d9
--- /dev/null
+++ b/debian/patches/bugfix/all/macvtap-zerocopy-set-SKBTX_DEV_ZEROCOPY-only-when-sk.patch
@@ -0,0 +1,48 @@
+From: Jason Wang <jasowang@redhat.com>
+Date: Wed, 2 May 2012 11:42:06 +0800
+Subject: [4/5] macvtap: zerocopy: set SKBTX_DEV_ZEROCOPY only when skb is
+ built successfully
+
+commit 01d6657b388438def19c8baaea28e742b6ed32ec upstream.
+
+Current the SKBTX_DEV_ZEROCOPY is set unconditionally after
+zerocopy_sg_from_iovec(), this would lead NULL pointer when macvtap
+fails to build zerocopy skb because destructor_arg was not
+initialized. Solve this by set this flag after the skb were built
+successfully.
+
+Signed-off-by: Jason Wang <jasowang@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+---
+ drivers/net/macvtap.c |    9 +++++----
+ 1 file changed, 5 insertions(+), 4 deletions(-)
+
+diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
+index 9ab182a..a4ff694 100644
+--- a/drivers/net/macvtap.c
++++ b/drivers/net/macvtap.c
+@@ -699,10 +699,9 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
+ 	if (!skb)
+ 		goto err;
+ 
+-	if (zerocopy) {
++	if (zerocopy)
+ 		err = zerocopy_sg_from_iovec(skb, iv, vnet_hdr_len, count);
+-		skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
+-	} else
++	else
+ 		err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len,
+ 						   len);
+ 	if (err)
+@@ -721,8 +720,10 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
+ 	rcu_read_lock_bh();
+ 	vlan = rcu_dereference_bh(q->vlan);
+ 	/* copy skb_ubuf_info for callback when skb has no error */
+-	if (zerocopy)
++	if (zerocopy) {
+ 		skb_shinfo(skb)->destructor_arg = m->msg_control;
++		skb_shinfo(skb)->tx_flags |= SKBTX_DEV_ZEROCOPY;
++	}
+ 	if (vlan)
+ 		macvlan_start_xmit(skb, vlan->dev);
+ 	else
diff --git a/debian/patches/bugfix/all/macvtap-zerocopy-validate-vectors-before-building-sk.patch b/debian/patches/bugfix/all/macvtap-zerocopy-validate-vectors-before-building-sk.patch
new file mode 100644
index 000000000000..b9f8ce82ce7c
--- /dev/null
+++ b/debian/patches/bugfix/all/macvtap-zerocopy-validate-vectors-before-building-sk.patch
@@ -0,0 +1,78 @@
+From: Jason Wang <jasowang@redhat.com>
+Date: Wed, 2 May 2012 11:42:15 +0800
+Subject: macvtap: zerocopy: validate vectors before building skb
+
+commit b92946e2919134ebe2a4083e4302236295ea2a73 upstream.
+
+There're several reasons that the vectors need to be validated:
+
+- Return error when caller provides vectors whose num is greater than UIO_MAXIOV.
+- Linearize part of skb when userspace provides vectors grater than MAX_SKB_FRAGS.
+- Return error when userspace provides vectors whose total length may exceed
+- MAX_SKB_FRAGS * PAGE_SIZE.
+
+Signed-off-by: Jason Wang <jasowang@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+---
+ drivers/net/macvtap.c |   25 +++++++++++++++++++++----
+ 1 file changed, 21 insertions(+), 4 deletions(-)
+
+diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
+index a4ff694..163559c 100644
+--- a/drivers/net/macvtap.c
++++ b/drivers/net/macvtap.c
+@@ -529,9 +529,10 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
+ 		}
+ 		base = (unsigned long)from->iov_base + offset;
+ 		size = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
++		if (i + size > MAX_SKB_FRAGS)
++			return -EMSGSIZE;
+ 		num_pages = get_user_pages_fast(base, size, 0, &page[i]);
+-		if ((num_pages != size) ||
+-		    (num_pages > MAX_SKB_FRAGS - skb_shinfo(skb)->nr_frags)) {
++		if (num_pages != size) {
+ 			for (i = 0; i < num_pages; i++)
+ 				put_page(page[i]);
+ 			return -EFAULT;
+@@ -651,7 +652,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
+ 	int err;
+ 	struct virtio_net_hdr vnet_hdr = { 0 };
+ 	int vnet_hdr_len = 0;
+-	int copylen;
++	int copylen = 0;
+ 	bool zerocopy = false;
+ 
+ 	if (q->flags & IFF_VNET_HDR) {
+@@ -680,15 +681,31 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
+ 	if (unlikely(len < ETH_HLEN))
+ 		goto err;
+ 
++	err = -EMSGSIZE;
++	if (unlikely(count > UIO_MAXIOV))
++		goto err;
++
+ 	if (m && m->msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY))
+ 		zerocopy = true;
+ 
+ 	if (zerocopy) {
++		/* Userspace may produce vectors with count greater than
++		 * MAX_SKB_FRAGS, so we need to linearize parts of the skb
++		 * to let the rest of data to be fit in the frags.
++		 */
++		if (count > MAX_SKB_FRAGS) {
++			copylen = iov_length(iv, count - MAX_SKB_FRAGS);
++			if (copylen < vnet_hdr_len)
++				copylen = 0;
++			else
++				copylen -= vnet_hdr_len;
++		}
+ 		/* There are 256 bytes to be copied in skb, so there is enough
+ 		 * room for skb expand head in case it is used.
+ 		 * The rest buffer is mapped from userspace.
+ 		 */
+-		copylen = vnet_hdr.hdr_len;
++		if (copylen < vnet_hdr.hdr_len)
++			copylen = vnet_hdr.hdr_len;
+ 		if (!copylen)
+ 			copylen = GOODCOPY_LEN;
+ 	} else
diff --git a/debian/patches/series b/debian/patches/series
index 27fa4d5cf348..420d198cf1ad 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -318,3 +318,9 @@ debian/avoid-ABI-change-for-hidepid.patch
 bugfix/all/NFSv4-Reduce-the-footprint-of-the-idmapper.patch
 bugfix/all/NFSv4-Further-reduce-the-footprint-of-the-idmapper.patch
 bugfix/all/net-sock-validate-data_len-before-allocating-skb-in-.patch
+
+bugfix/all/macvtap-zerocopy-fix-offset-calculation-when-buildin.patch
+bugfix/all/macvtap-zerocopy-fix-truesize-underestimation.patch
+bugfix/all/macvtap-zerocopy-put-page-when-fail-to-get-all-reque.patch
+bugfix/all/macvtap-zerocopy-set-SKBTX_DEV_ZEROCOPY-only-when-sk.patch
+bugfix/all/macvtap-zerocopy-validate-vectors-before-building-sk.patch