[linux] 01/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 19:27:45 UTC 2015
This is an automated email from the git hooks/post-receive script.
benh pushed a commit to branch jessie-security
in repository linux.
commit 9477aa3b0b67398b248f966b14689b20836c8a92
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 | 6 +
.../bugfix/all/xen-add-ring_copy_request.patch | 52 +++++++++
...-only-read-request-operation-from-shared-.patch | 48 ++++++++
...-read-from-indirect-descriptors-only-once.patch | 63 +++++++++++
...-don-t-use-last-request-to-determine-mini.patch | 35 ++++++
...-netback-use-ring_copy_request-throughout.patch | 126 +++++++++++++++++++++
...-save-xen_pci_op-commands-before-processi.patch | 77 +++++++++++++
debian/patches/series | 6 +
8 files changed, 413 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index 045f42b..e07a936 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+linux (3.16.7-ckt20-1+deb8u2) UNRELEASED; urgency=medium
+
+ * [xen] Fix race conditions in back-end drivers (CVE-2015-8550, XSA-155)
+
+ -- Ben Hutchings <ben at decadent.org.uk> Sun, 27 Dec 2015 14:20:54 +0000
+
linux (3.16.7-ckt20-1+deb8u1) jessie-security; urgency=medium
[ Salvatore Bonaccorso ]
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..c7b12ce
--- /dev/null
+++ b/debian/patches/bugfix/all/xen-blkback-only-read-request-operation-from-shared-.patch
@@ -0,0 +1,48 @@
+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>
+---
+ 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
+@@ -391,8 +391,8 @@ static inline void blkif_get_x86_32_req(
+ struct blkif_x86_32_request *src)
+ {
+ int i, n = BLKIF_MAX_SEGMENTS_PER_REQUEST, j;
+- dst->operation = src->operation;
+- switch (src->operation) {
++ dst->operation = READ_ONCE(src->operation);
++ switch (dst->operation) {
+ case BLKIF_OP_READ:
+ case BLKIF_OP_WRITE:
+ case BLKIF_OP_WRITE_BARRIER:
+@@ -439,8 +439,8 @@ static inline void blkif_get_x86_64_req(
+ struct blkif_x86_64_request *src)
+ {
+ int i, n = BLKIF_MAX_SEGMENTS_PER_REQUEST, j;
+- dst->operation = src->operation;
+- switch (src->operation) {
++ dst->operation = READ_ONCE(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-blkback-read-from-indirect-descriptors-only-once.patch b/debian/patches/bugfix/all/xen-blkback-read-from-indirect-descriptors-only-once.patch
new file mode 100644
index 0000000..de74707
--- /dev/null
+++ b/debian/patches/bugfix/all/xen-blkback-read-from-indirect-descriptors-only-once.patch
@@ -0,0 +1,63 @@
+From: =?UTF-8?q?Roger=20Pau=20Monn=C3=A9?= <roger.pau at citrix.com>
+Date: Tue, 3 Nov 2015 16:40:43 +0000
+Subject: [5/7] xen-blkback: read from indirect descriptors only once
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+Origin: https://git.kernel.org/linus/18779149101c0dd43ded43669ae2a92d21b6f9cb
+
+Since indirect descriptors are in memory shared with the frontend, the
+frontend could alter the first_sect and last_sect values after they have
+been validated but before they are recorded in the request. This may
+result in I/O requests that overflow the foreign page, possibly
+overwriting local pages when the I/O request is executed.
+
+When parsing indirect descriptors, only read first_sect and last_sect
+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: For 4.3, s/XEN_PAGE_SIZE/PAGE_SIZE/]
+---
+ drivers/block/xen-blkback/blkback.c | 15 ++++++++++-----
+ 1 file changed, 10 insertions(+), 5 deletions(-)
+
+--- a/drivers/block/xen-blkback/blkback.c
++++ b/drivers/block/xen-blkback/blkback.c
+@@ -861,6 +861,8 @@ static int xen_blkbk_parse_indirect(stru
+ goto unmap;
+
+ for (n = 0, i = 0; n < nseg; n++) {
++ uint8_t first_sect, last_sect;
++
+ if ((n % SEGS_PER_INDIRECT_FRAME) == 0) {
+ /* Map indirect segments */
+ if (segments)
+@@ -868,15 +870,18 @@ static int xen_blkbk_parse_indirect(stru
+ segments = kmap_atomic(pages[n/SEGS_PER_INDIRECT_FRAME]->page);
+ }
+ i = n % SEGS_PER_INDIRECT_FRAME;
++
+ pending_req->segments[n]->gref = segments[i].gref;
+- seg[n].nsec = segments[i].last_sect -
+- segments[i].first_sect + 1;
+- seg[n].offset = (segments[i].first_sect << 9);
+- if ((segments[i].last_sect >= (PAGE_SIZE >> 9)) ||
+- (segments[i].last_sect < segments[i].first_sect)) {
++
++ first_sect = READ_ONCE(segments[i].first_sect);
++ last_sect = READ_ONCE(segments[i].last_sect);
++ if (last_sect >= (PAGE_SIZE >> 9) || last_sect < first_sect) {
+ rc = -EINVAL;
+ goto unmap;
+ }
++
++ seg[n].nsec = last_sect - first_sect + 1;
++ seg[n].offset = first_sect << 9;
+ preq->nr_sects += seg[n].nsec;
+ }
+
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..4322399
--- /dev/null
+++ b/debian/patches/bugfix/all/xen-netback-don-t-use-last-request-to-determine-mini.patch
@@ -0,0 +1,35 @@
+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>
+---
+ 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
+@@ -809,9 +809,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(&queue->tx, queue->tx.req_cons)->size;
+- max_burst = min(max_burst, 131072UL);
+- max_burst = max(max_burst, queue->credit_bytes);
++ max_burst = max(131072UL, queue->credit_bytes);
+
+ /* Take care that adding a new chunk of credit doesn't wrap to zero. */
+ max_credit = queue->remaining_credit + queue->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..d4bb7e1
--- /dev/null
+++ b/debian/patches/bugfix/all/xen-netback-use-ring_copy_request-throughout.patch
@@ -0,0 +1,126 @@
+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>
+---
+ 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
+@@ -288,18 +288,18 @@ static struct xenvif_rx_meta *get_next_r
+ struct netrx_pending_operations *npo)
+ {
+ struct xenvif_rx_meta *meta;
+- struct xen_netif_rx_request *req;
++ struct xen_netif_rx_request req;
+
+- req = RING_GET_REQUEST(&queue->rx, queue->rx.req_cons++);
++ RING_COPY_REQUEST(&queue->rx, queue->rx.req_cons++, &req);
+
+ meta = npo->meta + npo->meta_prod++;
+ meta->gso_type = XEN_NETIF_GSO_TYPE_NONE;
+ 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;
+ }
+@@ -450,7 +450,7 @@ static int xenvif_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 xenvif_rx_meta *meta;
+ unsigned char *data;
+ int head = 1;
+@@ -471,15 +471,15 @@ static int xenvif_gop_skb(struct sk_buff
+
+ /* Set up a GSO prefix descriptor, if necessary */
+ if ((1 << gso_type) & vif->gso_prefix_mask) {
+- req = RING_GET_REQUEST(&queue->rx, queue->rx.req_cons++);
++ RING_COPY_REQUEST(&queue->rx, queue->rx.req_cons++, &req);
+ meta = npo->meta + npo->meta_prod++;
+ meta->gso_type = gso_type;
+ meta->gso_size = skb_shinfo(skb)->gso_size;
+ meta->size = 0;
+- meta->id = req->id;
++ meta->id = req.id;
+ }
+
+- req = RING_GET_REQUEST(&queue->rx, queue->rx.req_cons++);
++ RING_COPY_REQUEST(&queue->rx, queue->rx.req_cons++, &req);
+ meta = npo->meta + npo->meta_prod++;
+
+ if ((1 << gso_type) & vif->gso_mask) {
+@@ -491,9 +491,9 @@ static int xenvif_gop_skb(struct sk_buff
+ }
+
+ 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)) {
+@@ -838,7 +838,7 @@ static void xenvif_tx_err(struct xenvif_
+ spin_unlock_irqrestore(&queue->response_lock, flags);
+ if (cons == end)
+ break;
+- txp = RING_GET_REQUEST(&queue->tx, cons++);
++ RING_COPY_REQUEST(&queue->tx, cons++, txp);
+ } while (1);
+ queue->tx.req_cons = cons;
+ }
+@@ -905,8 +905,7 @@ static int xenvif_count_requests(struct
+ if (drop_err)
+ txp = &dropped_tx;
+
+- memcpy(txp, RING_GET_REQUEST(&queue->tx, cons + slots),
+- sizeof(*txp));
++ RING_COPY_REQUEST(&queue->tx, cons + slots, txp);
+
+ /* If the guest submitted a frame >= 64 KiB then
+ * first->size overflowed and following slots will
+@@ -1258,8 +1257,7 @@ static int xenvif_get_extras(struct xenv
+ return -EBADR;
+ }
+
+- memcpy(&extra, RING_GET_REQUEST(&queue->tx, cons),
+- sizeof(extra));
++ RING_COPY_REQUEST(&queue->tx, cons, &extra);
+ if (unlikely(!extra.type ||
+ extra.type >= XEN_NETIF_EXTRA_TYPE_MAX)) {
+ queue->tx.req_cons = ++cons;
+@@ -1395,7 +1393,7 @@ static void xenvif_tx_build_gops(struct
+
+ idx = queue->tx.req_cons;
+ rmb(); /* Ensure that we see the request before we copy it. */
+- memcpy(&txreq, RING_GET_REQUEST(&queue->tx, idx), sizeof(txreq));
++ RING_COPY_REQUEST(&queue->tx, idx, &txreq);
+
+ /* Credit-based scheduling. */
+ if (txreq.size > queue->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..101afe8
--- /dev/null
+++ b/debian/patches/bugfix/all/xen-pciback-save-xen_pci_op-commands-before-processi.patch
@@ -0,0 +1,77 @@
+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(-)
+
+diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h
+index 58e38d5..4d529f3 100644
+--- 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 {
+diff --git a/drivers/xen/xen-pciback/pciback_ops.c b/drivers/xen/xen-pciback/pciback_ops.c
+index c4a0666..a0e0e3e 100644
+--- a/drivers/xen/xen-pciback/pciback_ops.c
++++ b/drivers/xen/xen-pciback/pciback_ops.c
+@@ -298,9 +298,11 @@ void xen_pcibk_do_op(struct work_struct *data)
+ 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)
+@@ -342,6 +344,17 @@ void xen_pcibk_do_op(struct work_struct *data)
+ 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 aafb392..44e3946 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -650,3 +650,9 @@ bugfix/all/unix-avoid-use-after-free-in-ep_remove_wait_queue.patch
debian/af_unix-avoid-abi-changes.patch
bugfix/all/btrfs-fix-truncation-of-compressed-and-inlined-exten.patch
bugfix/all/net-add-validation-for-the-socket-syscall-protocol.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-blkback-read-from-indirect-descriptors-only-once.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