CVE-2012-2119: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: Buffer overflow in the macvtap device driver in the Linux kernel before 3.4.5, when running in certain configurations, allows privileged KVM guest users to cause a denial of service (crash) via a long descriptor with a long vector length.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
Published: 2013-01-22T23:55Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: drivers/net/macvtap.c
Commit date: 2012-05-02 18:22:21 +0300
Commit message: macvtap: zerocopy: validate vectors before building skb

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>

Diff: diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a4ff694cea228..163559c169883 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -527,13 +527,14 @@ static int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *from,
 			++from;
 			continue;
 		}
 		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;
 		}
 		truesize = size * PAGE_SIZE;
@@ -649,11 +650,11 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
 	struct macvlan_dev *vlan;
 	unsigned long len = total_len;
 	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) {
 		vnet_hdr_len = q->vnet_hdr_sz;
 
@@ -678,19 +679,35 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
 
 	err = -EINVAL;
 	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
 		copylen = len;
 

Vulnerable code: 		if ((num_pages != size) ||
		    (num_pages > MAX_SKB_FRAGS - skb_shinfo(skb)->nr_frags)) {
	int copylen;
		copylen = vnet_hdr.hdr_len;
Fixed code: 		if (i + size > MAX_SKB_FRAGS)
			return -EMSGSIZE;
		if (num_pages != size) {
	int copylen = 0;
	err = -EMSGSIZE;
	if (unlikely(count > UIO_MAXIOV))
		goto err;

		/* 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;
		}
		if (copylen < vnet_hdr.hdr_len)
			copylen = vnet_hdr.hdr_len;