From 4837a1a51638ef1719bf8149591a57e7207db41a Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Mon, 23 May 2016 00:44:57 -0600 Subject: [PATCH 1/2] xen/blkif: avoid double access to any shared ring request fields Commit f9e98e5d7a ("xen/blkif: Avoid double access to src->nr_segments") didn't go far enough: src->operation is also being used twice. And nothing was done to prevent the compiler from using the source side of the copy done by blk_get_request() (granted that's very unlikely). Move the barrier()s up, and add another one to blk_get_request(). Note that for completing XSA-155, the barrier() getting added to blk_get_request() would suffice, and hence the changes to xen_blkif.h are more like just cleanup. And since, as said, the unpatched code getting compiled to something vulnerable is very unlikely (and not observed in practice), this isn't being viewed as a new security issue. Signed-off-by: Jan Beulich Reviewed-by: Stefano Stabellini Signed-off-by: Stefano Stabellini --- hw/block/xen_blkif.h | 12 ++++++------ hw/block/xen_disk.c | 2 ++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h index c68487cb31..e3b133b9f8 100644 --- a/hw/block/xen_blkif.h +++ b/hw/block/xen_blkif.h @@ -79,14 +79,14 @@ static inline void blkif_get_x86_32_req(blkif_request_t *dst, blkif_x86_32_reque dst->handle = src->handle; dst->id = src->id; dst->sector_number = src->sector_number; - if (src->operation == BLKIF_OP_DISCARD) { + /* Prevent the compiler from using src->... instead. */ + barrier(); + if (dst->operation == BLKIF_OP_DISCARD) { struct blkif_request_discard *s = (void *)src; struct blkif_request_discard *d = (void *)dst; d->nr_sectors = s->nr_sectors; return; } - /* prevent the compiler from optimizing the code and using src->nr_segments instead */ - barrier(); if (n > dst->nr_segments) n = dst->nr_segments; for (i = 0; i < n; i++) @@ -102,14 +102,14 @@ static inline void blkif_get_x86_64_req(blkif_request_t *dst, blkif_x86_64_reque dst->handle = src->handle; dst->id = src->id; dst->sector_number = src->sector_number; - if (src->operation == BLKIF_OP_DISCARD) { + /* Prevent the compiler from using src->... instead. */ + barrier(); + if (dst->operation == BLKIF_OP_DISCARD) { struct blkif_request_discard *s = (void *)src; struct blkif_request_discard *d = (void *)dst; d->nr_sectors = s->nr_sectors; return; } - /* prevent the compiler from optimizing the code and using src->nr_segments instead */ - barrier(); if (n > dst->nr_segments) n = dst->nr_segments; for (i = 0; i < n; i++) diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 064c116a7c..cf57814fb6 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -679,6 +679,8 @@ static int blk_get_request(struct XenBlkDev *blkdev, struct ioreq *ioreq, RING_I RING_GET_REQUEST(&blkdev->rings.x86_64_part, rc)); break; } + /* Prevent the compiler from accessing the on-ring fields instead. */ + barrier(); return 0; } From b1b23e5bbfb66d9401e2c2b0646fb721d94a3f83 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 24 May 2016 16:27:18 +0100 Subject: [PATCH 2/2] xen: Clean up includes Clean up includes so that osdep.h is included first and headers which it implies are not included manually. This commit was created with scripts/clean-includes. Signed-off-by: Peter Maydell Reviewed-by: Stefano Stabellini Signed-off-by: Stefano Stabellini --- hw/usb/xen-usb.c | 9 +++------ include/hw/xen/xen.h | 1 - 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c index 664df04181..8fa47edd9a 100644 --- a/hw/usb/xen-usb.c +++ b/hw/usb/xen-usb.c @@ -19,13 +19,10 @@ * GNU GPL, version 2 or (at your option) any later version. */ -#include -#include -#include -#include -#include - #include "qemu/osdep.h" +#include +#include + #include "qemu-common.h" #include "qemu/config-file.h" #include "hw/sysbus.h" diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h index 6365483473..b2cd992430 100644 --- a/include/hw/xen/xen.h +++ b/include/hw/xen/xen.h @@ -8,7 +8,6 @@ */ #include "qemu-common.h" -#include "qemu/typedefs.h" #include "exec/cpu-common.h" #include "hw/irq.h"