[linux] 02/05: [xen] Fix race conditions in back-end drivers (CVE-2015-8550, XSA-155)
debian-kernel at lists.debian.org
debian-kernel at lists.debian.org
Sun Dec 27 22:46:32 UTC 2015
This is an automated email from the git hooks/post-receive script.
benh pushed a commit to branch wheezy-security
in repository linux.
commit fabe5ae4e460932d469205fc0e50245ed209f84e
Author: Ben Hutchings <ben at decadent.org.uk>
Date: Sun Dec 27 14:22:58 2015 +0000
[xen] Fix race conditions in back-end drivers (CVE-2015-8550, XSA-155)
---
debian/changelog | 1 +
.../bugfix/all/xen-add-ring_copy_request.patch | 52 +++++++++
...-only-read-request-operation-from-shared-.patch | 57 +++++++++
...-don-t-use-last-request-to-determine-mini.patch | 36 ++++++
...-netback-use-ring_copy_request-throughout.patch | 127 +++++++++++++++++++++
...-save-xen_pci_op-commands-before-processi.patch | 73 ++++++++++++
debian/patches/series | 5 +
7 files changed, 351 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index c6aceb1..eb04e0c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,6 +1,7 @@
linux (3.2.73-2+deb7u2) UNRELEASED; urgency=medium
* net: add validation for the socket syscall protocol argument (CVE-2015-8543)
+ * [xen] Fix race conditions in back-end drivers (CVE-2015-8550, XSA-155)
-- Ben Hutchings <ben at decadent.org.uk> Sun, 27 Dec 2015 19:23:43 +0000
diff --git a/debian/patches/bugfix/all/xen-add-ring_copy_request.patch b/debian/patches/bugfix/all/xen-add-ring_copy_request.patch
new file mode 100644
index 0000000..51e9546
--- /dev/null
+++ b/debian/patches/bugfix/all/xen-add-ring_copy_request.patch
@@ -0,0 +1,52 @@
+From: David Vrabel <david.vrabel at citrix.com>
+Date: Fri, 30 Oct 2015 14:58:08 +0000
+Subject: [1/7] xen: Add RING_COPY_REQUEST()
+Origin: https://git.kernel.org/linus/454d5d882c7e412b840e3c99010fe81a9862f6fb
+
+Using RING_GET_REQUEST() on a shared ring is easy to use incorrectly
+(i.e., by not considering that the other end may alter the data in the
+shared ring while it is being inspected). Safe usage of a request
+generally requires taking a local copy.
+
+Provide a RING_COPY_REQUEST() macro to use instead of
+RING_GET_REQUEST() and an open-coded memcpy(). This takes care of
+ensuring that the copy is done correctly regardless of any possible
+compiler optimizations.
+
+Use a volatile source to prevent the compiler from reordering or
+omitting the copy.
+
+This is part of XSA155.
+
+CC: stable at vger.kernel.org
+Signed-off-by: David Vrabel <david.vrabel at citrix.com>
+Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
+---
+ include/xen/interface/io/ring.h | 14 ++++++++++++++
+ 1 file changed, 14 insertions(+)
+
+diff --git a/include/xen/interface/io/ring.h b/include/xen/interface/io/ring.h
+index 7d28aff..7dc685b4 100644
+--- a/include/xen/interface/io/ring.h
++++ b/include/xen/interface/io/ring.h
+@@ -181,6 +181,20 @@ struct __name##_back_ring { \
+ #define RING_GET_REQUEST(_r, _idx) \
+ (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req))
+
++/*
++ * Get a local copy of a request.
++ *
++ * Use this in preference to RING_GET_REQUEST() so all processing is
++ * done on a local copy that cannot be modified by the other end.
++ *
++ * Note that https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145 may cause this
++ * to be ineffective where _req is a struct which consists of only bitfields.
++ */
++#define RING_COPY_REQUEST(_r, _idx, _req) do { \
++ /* Use volatile to force the copy into _req. */ \
++ *(_req) = *(volatile typeof(_req))RING_GET_REQUEST(_r, _idx); \
++} while (0)
++
+ #define RING_GET_RESPONSE(_r, _idx) \
+ (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].rsp))
+
diff --git a/debian/patches/bugfix/all/xen-blkback-only-read-request-operation-from-shared-.patch b/debian/patches/bugfix/all/xen-blkback-only-read-request-operation-from-shared-.patch
new file mode 100644
index 0000000..c28f1ae
--- /dev/null
+++ b/debian/patches/bugfix/all/xen-blkback-only-read-request-operation-from-shared-.patch
@@ -0,0 +1,57 @@
+From: =?UTF-8?q?Roger=20Pau=20Monn=C3=A9?= <roger.pau at citrix.com>
+Date: Tue, 3 Nov 2015 16:34:09 +0000
+Subject: [4/7] xen-blkback: only read request operation from shared ring once
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+Origin: https://git.kernel.org/linus/1f13d75ccb806260079e0679d55d9253e370ec8a
+
+A compiler may load a switch statement value multiple times, which could
+be bad when the value is in memory shared with the frontend.
+
+When converting a non-native request to a native one, ensure that
+src->operation is only loaded once by using READ_ONCE().
+
+This is part of XSA155.
+
+CC: stable at vger.kernel.org
+Signed-off-by: Roger Pau Monné <roger.pau at citrix.com>
+Signed-off-by: David Vrabel <david.vrabel at citrix.com>
+Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
+[bwh: Backported to 3.2:
+ - s/READ_ONCE/ACCESS_ONCE/
+ - Adjust context]
+---
+ drivers/block/xen-blkback/common.h | 8 ++++----
+ 1 file changed, 4 insertions(+), 4 deletions(-)
+
+--- a/drivers/block/xen-blkback/common.h
++++ b/drivers/block/xen-blkback/common.h
+@@ -238,11 +238,11 @@ static inline void blkif_get_x86_32_req(
+ struct blkif_x86_32_request *src)
+ {
+ int i, n = BLKIF_MAX_SEGMENTS_PER_REQUEST;
+- dst->operation = src->operation;
++ dst->operation = ACCESS_ONCE(src->operation);
+ dst->nr_segments = src->nr_segments;
+ dst->handle = src->handle;
+ dst->id = src->id;
+- switch (src->operation) {
++ switch (dst->operation) {
+ case BLKIF_OP_READ:
+ case BLKIF_OP_WRITE:
+ case BLKIF_OP_WRITE_BARRIER:
+@@ -267,11 +267,11 @@ static inline void blkif_get_x86_64_req(
+ struct blkif_x86_64_request *src)
+ {
+ int i, n = BLKIF_MAX_SEGMENTS_PER_REQUEST;
+- dst->operation = src->operation;
++ dst->operation = ACCESS_ONCE(src->operation);
+ dst->nr_segments = src->nr_segments;
+ dst->handle = src->handle;
+ dst->id = src->id;
+- switch (src->operation) {
++ switch (dst->operation) {
+ case BLKIF_OP_READ:
+ case BLKIF_OP_WRITE:
+ case BLKIF_OP_WRITE_BARRIER:
diff --git a/debian/patches/bugfix/all/xen-netback-don-t-use-last-request-to-determine-mini.patch b/debian/patches/bugfix/all/xen-netback-don-t-use-last-request-to-determine-mini.patch
new file mode 100644
index 0000000..5be0784
--- /dev/null
+++ b/debian/patches/bugfix/all/xen-netback-don-t-use-last-request-to-determine-mini.patch
@@ -0,0 +1,36 @@
+From: David Vrabel <david.vrabel at citrix.com>
+Date: Fri, 30 Oct 2015 15:16:01 +0000
+Subject: [2/7] xen-netback: don't use last request to determine minimum Tx
+ credit
+Origin: https://git.kernel.org/linus/0f589967a73f1f30ab4ac4dd9ce0bb399b4d6357
+
+The last from guest transmitted request gives no indication about the
+minimum amount of credit that the guest might need to send a packet
+since the last packet might have been a small one.
+
+Instead allow for the worst case 128 KiB packet.
+
+This is part of XSA155.
+
+CC: stable at vger.kernel.org
+Reviewed-by: Wei Liu <wei.liu2 at citrix.com>
+Signed-off-by: David Vrabel <david.vrabel at citrix.com>
+Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
+[bwh: Backported to 3.2: s/queue/vif/g]
+---
+ drivers/net/xen-netback/netback.c | 4 +---
+ 1 file changed, 1 insertion(+), 3 deletions(-)
+
+--- a/drivers/net/xen-netback/netback.c
++++ b/drivers/net/xen-netback/netback.c
+@@ -864,9 +864,7 @@ static void tx_add_credit(struct xenvif
+ * Allow a burst big enough to transmit a jumbo packet of up to 128kB.
+ * Otherwise the interface can seize up due to insufficient credit.
+ */
+- max_burst = RING_GET_REQUEST(&vif->tx, vif->tx.req_cons)->size;
+- max_burst = min(max_burst, 131072UL);
+- max_burst = max(max_burst, vif->credit_bytes);
++ max_burst = max(131072UL, vif->credit_bytes);
+
+ /* Take care that adding a new chunk of credit doesn't wrap to zero. */
+ max_credit = vif->remaining_credit + vif->credit_bytes;
diff --git a/debian/patches/bugfix/all/xen-netback-use-ring_copy_request-throughout.patch b/debian/patches/bugfix/all/xen-netback-use-ring_copy_request-throughout.patch
new file mode 100644
index 0000000..594da33
--- /dev/null
+++ b/debian/patches/bugfix/all/xen-netback-use-ring_copy_request-throughout.patch
@@ -0,0 +1,127 @@
+From: David Vrabel <david.vrabel at citrix.com>
+Date: Fri, 30 Oct 2015 15:17:06 +0000
+Subject: [3/7] xen-netback: use RING_COPY_REQUEST() throughout
+Origin: https://git.kernel.org/linus/68a33bfd8403e4e22847165d149823a2e0e67c9c
+
+Instead of open-coding memcpy()s and directly accessing Tx and Rx
+requests, use the new RING_COPY_REQUEST() that ensures the local copy
+is correct.
+
+This is more than is strictly necessary for guest Rx requests since
+only the id and gref fields are used and it is harmless if the
+frontend modifies these.
+
+This is part of XSA155.
+
+CC: stable at vger.kernel.org
+Reviewed-by: Wei Liu <wei.liu2 at citrix.com>
+Signed-off-by: David Vrabel <david.vrabel at citrix.com>
+Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
+[bwh: Backported to 3.2:
+ - s/queue/vif/g
+ - Adjust context]
+---
+ drivers/net/xen-netback/netback.c | 30 ++++++++++++++----------------
+ 1 file changed, 14 insertions(+), 16 deletions(-)
+
+--- a/drivers/net/xen-netback/netback.c
++++ b/drivers/net/xen-netback/netback.c
+@@ -406,17 +406,17 @@ static struct netbk_rx_meta *get_next_rx
+ struct netrx_pending_operations *npo)
+ {
+ struct netbk_rx_meta *meta;
+- struct xen_netif_rx_request *req;
++ struct xen_netif_rx_request req;
+
+- req = RING_GET_REQUEST(&vif->rx, vif->rx.req_cons++);
++ RING_COPY_REQUEST(&vif->rx, vif->rx.req_cons++, &req);
+
+ meta = npo->meta + npo->meta_prod++;
+ meta->gso_size = 0;
+ meta->size = 0;
+- meta->id = req->id;
++ meta->id = req.id;
+
+ npo->copy_off = 0;
+- npo->copy_gref = req->gref;
++ npo->copy_gref = req.gref;
+
+ return meta;
+ }
+@@ -518,7 +518,7 @@ static int netbk_gop_skb(struct sk_buff
+ struct xenvif *vif = netdev_priv(skb->dev);
+ int nr_frags = skb_shinfo(skb)->nr_frags;
+ int i;
+- struct xen_netif_rx_request *req;
++ struct xen_netif_rx_request req;
+ struct netbk_rx_meta *meta;
+ unsigned char *data;
+ int head = 1;
+@@ -528,14 +528,14 @@ static int netbk_gop_skb(struct sk_buff
+
+ /* Set up a GSO prefix descriptor, if necessary */
+ if (skb_shinfo(skb)->gso_size && vif->gso_prefix) {
+- req = RING_GET_REQUEST(&vif->rx, vif->rx.req_cons++);
++ RING_COPY_REQUEST(&vif->rx, vif->rx.req_cons++, &req);
+ meta = npo->meta + npo->meta_prod++;
+ meta->gso_size = skb_shinfo(skb)->gso_size;
+ meta->size = 0;
+- meta->id = req->id;
++ meta->id = req.id;
+ }
+
+- req = RING_GET_REQUEST(&vif->rx, vif->rx.req_cons++);
++ RING_COPY_REQUEST(&vif->rx, vif->rx.req_cons++, &req);
+ meta = npo->meta + npo->meta_prod++;
+
+ if (!vif->gso_prefix)
+@@ -544,9 +544,9 @@ static int netbk_gop_skb(struct sk_buff
+ meta->gso_size = 0;
+
+ meta->size = 0;
+- meta->id = req->id;
++ meta->id = req.id;
+ npo->copy_off = 0;
+- npo->copy_gref = req->gref;
++ npo->copy_gref = req.gref;
+
+ data = skb->data;
+ while (data < skb_tail_pointer(skb)) {
+@@ -890,7 +890,7 @@ static void netbk_tx_err(struct xenvif *
+ make_tx_response(vif, txp, XEN_NETIF_RSP_ERROR);
+ if (cons == end)
+ break;
+- txp = RING_GET_REQUEST(&vif->tx, cons++);
++ RING_COPY_REQUEST(&vif->tx, cons++, txp);
+ } while (1);
+ vif->tx.req_cons = cons;
+ xen_netbk_check_rx_xenvif(vif);
+@@ -957,8 +957,7 @@ static int netbk_count_requests(struct x
+ if (drop_err)
+ txp = &dropped_tx;
+
+- memcpy(txp, RING_GET_REQUEST(&vif->tx, cons + slots),
+- sizeof(*txp));
++ RING_COPY_REQUEST(&vif->tx, cons + slots, txp);
+
+ /* If the guest submitted a frame >= 64 KiB then
+ * first->size overflowed and following slots will
+@@ -1246,8 +1245,7 @@ static int xen_netbk_get_extras(struct x
+ return -EBADR;
+ }
+
+- memcpy(&extra, RING_GET_REQUEST(&vif->tx, cons),
+- sizeof(extra));
++ RING_COPY_REQUEST(&vif->tx, cons, &extra);
+ if (unlikely(!extra.type ||
+ extra.type >= XEN_NETIF_EXTRA_TYPE_MAX)) {
+ vif->tx.req_cons = ++cons;
+@@ -1442,7 +1440,7 @@ static unsigned xen_netbk_tx_build_gops(
+
+ idx = vif->tx.req_cons;
+ rmb(); /* Ensure that we see the request before we copy it. */
+- memcpy(&txreq, RING_GET_REQUEST(&vif->tx, idx), sizeof(txreq));
++ RING_COPY_REQUEST(&vif->tx, idx, &txreq);
+
+ /* Credit-based scheduling. */
+ if (txreq.size > vif->remaining_credit &&
diff --git a/debian/patches/bugfix/all/xen-pciback-save-xen_pci_op-commands-before-processi.patch b/debian/patches/bugfix/all/xen-pciback-save-xen_pci_op-commands-before-processi.patch
new file mode 100644
index 0000000..2d5a29d
--- /dev/null
+++ b/debian/patches/bugfix/all/xen-pciback-save-xen_pci_op-commands-before-processi.patch
@@ -0,0 +1,73 @@
+From: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
+Date: Mon, 16 Nov 2015 12:40:48 -0500
+Subject: [7/7] xen/pciback: Save xen_pci_op commands before processing it
+Origin: https://git.kernel.org/linus/8135cf8b092723dbfcc611fe6fdcb3a36c9951c5
+
+Double fetch vulnerabilities that happen when a variable is
+fetched twice from shared memory but a security check is only
+performed the first time.
+
+The xen_pcibk_do_op function performs a switch statements on the op->cmd
+value which is stored in shared memory. Interestingly this can result
+in a double fetch vulnerability depending on the performed compiler
+optimization.
+
+This patch fixes it by saving the xen_pci_op command before
+processing it. We also use 'barrier' to make sure that the
+compiler does not perform any optimization.
+
+This is part of XSA155.
+
+CC: stable at vger.kernel.org
+Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
+Signed-off-by: Jan Beulich <JBeulich at suse.com>
+Signed-off-by: David Vrabel <david.vrabel at citrix.com>
+Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk at oracle.com>
+---
+ drivers/xen/xen-pciback/pciback.h | 1 +
+ drivers/xen/xen-pciback/pciback_ops.c | 15 ++++++++++++++-
+ 2 files changed, 15 insertions(+), 1 deletion(-)
+
+--- a/drivers/xen/xen-pciback/pciback.h
++++ b/drivers/xen/xen-pciback/pciback.h
+@@ -37,6 +37,7 @@ struct xen_pcibk_device {
+ struct xen_pci_sharedinfo *sh_info;
+ unsigned long flags;
+ struct work_struct op_work;
++ struct xen_pci_op op;
+ };
+
+ struct xen_pcibk_dev_data {
+--- a/drivers/xen/xen-pciback/pciback_ops.c
++++ b/drivers/xen/xen-pciback/pciback_ops.c
+@@ -296,9 +296,11 @@ void xen_pcibk_do_op(struct work_struct
+ container_of(data, struct xen_pcibk_device, op_work);
+ struct pci_dev *dev;
+ struct xen_pcibk_dev_data *dev_data = NULL;
+- struct xen_pci_op *op = &pdev->sh_info->op;
++ struct xen_pci_op *op = &pdev->op;
+ int test_intx = 0;
+
++ *op = pdev->sh_info->op;
++ barrier();
+ dev = xen_pcibk_get_pci_dev(pdev, op->domain, op->bus, op->devfn);
+
+ if (dev == NULL)
+@@ -340,6 +342,17 @@ void xen_pcibk_do_op(struct work_struct
+ if ((dev_data->enable_intx != test_intx))
+ xen_pcibk_control_isr(dev, 0 /* no reset */);
+ }
++ pdev->sh_info->op.err = op->err;
++ pdev->sh_info->op.value = op->value;
++#ifdef CONFIG_PCI_MSI
++ if (op->cmd == XEN_PCI_OP_enable_msix && op->err == 0) {
++ unsigned int i;
++
++ for (i = 0; i < op->value; i++)
++ pdev->sh_info->op.msix_entries[i].vector =
++ op->msix_entries[i].vector;
++ }
++#endif
+ /* Tell the driver domain that we're done. */
+ wmb();
+ clear_bit(_XEN_PCIF_active, (unsigned long *)&pdev->sh_info->flags);
diff --git a/debian/patches/series b/debian/patches/series
index 1af65a2..6ceda71 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1172,3 +1172,8 @@ bugfix/all/splice-sendfile-at-once-fails-for-big-files.patch
bugfix/all/unix-avoid-use-after-free-in-ep_remove_wait_queue.patch
debian/af_unix-avoid-abi-changes.patch
bugfix/all/net-add-validation-for-the-socket-syscall-protocol-a.patch
+bugfix/all/xen-add-ring_copy_request.patch
+bugfix/all/xen-netback-don-t-use-last-request-to-determine-mini.patch
+bugfix/all/xen-netback-use-ring_copy_request-throughout.patch
+bugfix/all/xen-blkback-only-read-request-operation-from-shared-.patch
+bugfix/all/xen-pciback-save-xen_pci_op-commands-before-processi.patch
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/kernel/linux.git
More information about the Kernel-svn-changes
mailing list