diff --git a/debian/changelog b/debian/changelog index 1897afeb2..6de4d61f4 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 000000000..911076814 --- /dev/null +++ b/debian/patches/bugfix/all/macvtap-zerocopy-fix-offset-calculation-when-buildin.patch @@ -0,0 +1,61 @@ +From: Jason Wang +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 +Signed-off-by: Michael S. Tsirkin +--- + 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 000000000..75164f0a2 --- /dev/null +++ b/debian/patches/bugfix/all/macvtap-zerocopy-fix-truesize-underestimation.patch @@ -0,0 +1,41 @@ +From: Jason Wang +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 +Signed-off-by: Michael S. Tsirkin +--- + 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 000000000..1674eb84f --- /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 +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 +Signed-off-by: Michael S. Tsirkin +--- + 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 000000000..fb5c20256 --- /dev/null +++ b/debian/patches/bugfix/all/macvtap-zerocopy-set-SKBTX_DEV_ZEROCOPY-only-when-sk.patch @@ -0,0 +1,48 @@ +From: Jason Wang +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 +Signed-off-by: Michael S. Tsirkin +--- + 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 000000000..b9f8ce82c --- /dev/null +++ b/debian/patches/bugfix/all/macvtap-zerocopy-validate-vectors-before-building-sk.patch @@ -0,0 +1,78 @@ +From: Jason Wang +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 +Signed-off-by: Michael S. Tsirkin +--- + 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 27fa4d5cf..420d198cf 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