1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
| | Fix CVE-2017-10911:
https://xenbits.xen.org/xsa/advisory-216.html
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-10911
https://security-tracker.debian.org/tracker/CVE-2017-10911
Patch copied from Xen Security Advisory:
https://xenbits.xen.org/xsa/xsa216-qemuu.patch
From: Jan Beulich <jbeulich@suse.com>
Subject: xen/disk: don't leak stack data via response ring
Rather than constructing a local structure instance on the stack, fill
the fields directly on the shared ring, just like other (Linux)
backends do. Build on the fact that all response structure flavors are
actually identical (the old code did make this assumption too).
This is XSA-216.
Reported-by: Anthony Perard <anthony.perard@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
---
v2: Add QEMU_PACKED to fix handling 32-bit guests by 64-bit qemu.
--- a/hw/block/xen_blkif.h
+++ b/hw/block/xen_blkif.h
@@ -14,9 +14,6 @@
struct blkif_common_request {
char dummy;
};
-struct blkif_common_response {
- char dummy;
-};
/* i386 protocol version */
#pragma pack(push, 4)
@@ -36,13 +33,7 @@ struct blkif_x86_32_request_discard {
blkif_sector_t sector_number; /* start sector idx on disk (r/w only) */
uint64_t nr_sectors; /* # of contiguous sectors to discard */
};
-struct blkif_x86_32_response {
- uint64_t id; /* copied from request */
- uint8_t operation; /* copied from request */
- int16_t status; /* BLKIF_RSP_??? */
-};
typedef struct blkif_x86_32_request blkif_x86_32_request_t;
-typedef struct blkif_x86_32_response blkif_x86_32_response_t;
#pragma pack(pop)
/* x86_64 protocol version */
@@ -62,20 +53,14 @@ struct blkif_x86_64_request_discard {
blkif_sector_t sector_number; /* start sector idx on disk (r/w only) */
uint64_t nr_sectors; /* # of contiguous sectors to discard */
};
-struct blkif_x86_64_response {
- uint64_t __attribute__((__aligned__(8))) id;
- uint8_t operation; /* copied from request */
- int16_t status; /* BLKIF_RSP_??? */
-};
typedef struct blkif_x86_64_request blkif_x86_64_request_t;
-typedef struct blkif_x86_64_response blkif_x86_64_response_t;
DEFINE_RING_TYPES(blkif_common, struct blkif_common_request,
- struct blkif_common_response);
+ struct blkif_response);
DEFINE_RING_TYPES(blkif_x86_32, struct blkif_x86_32_request,
- struct blkif_x86_32_response);
+ struct blkif_response QEMU_PACKED);
DEFINE_RING_TYPES(blkif_x86_64, struct blkif_x86_64_request,
- struct blkif_x86_64_response);
+ struct blkif_response);
union blkif_back_rings {
blkif_back_ring_t native;
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -769,31 +769,30 @@ static int blk_send_response_one(struct
struct XenBlkDev *blkdev = ioreq->blkdev;
int send_notify = 0;
int have_requests = 0;
- blkif_response_t resp;
- void *dst;
-
- resp.id = ioreq->req.id;
- resp.operation = ioreq->req.operation;
- resp.status = ioreq->status;
+ blkif_response_t *resp;
/* Place on the response ring for the relevant domain. */
switch (blkdev->protocol) {
case BLKIF_PROTOCOL_NATIVE:
- dst = RING_GET_RESPONSE(&blkdev->rings.native, blkdev->rings.native.rsp_prod_pvt);
+ resp = RING_GET_RESPONSE(&blkdev->rings.native,
+ blkdev->rings.native.rsp_prod_pvt);
break;
case BLKIF_PROTOCOL_X86_32:
- dst = RING_GET_RESPONSE(&blkdev->rings.x86_32_part,
- blkdev->rings.x86_32_part.rsp_prod_pvt);
+ resp = RING_GET_RESPONSE(&blkdev->rings.x86_32_part,
+ blkdev->rings.x86_32_part.rsp_prod_pvt);
break;
case BLKIF_PROTOCOL_X86_64:
- dst = RING_GET_RESPONSE(&blkdev->rings.x86_64_part,
- blkdev->rings.x86_64_part.rsp_prod_pvt);
+ resp = RING_GET_RESPONSE(&blkdev->rings.x86_64_part,
+ blkdev->rings.x86_64_part.rsp_prod_pvt);
break;
default:
- dst = NULL;
return 0;
}
- memcpy(dst, &resp, sizeof(resp));
+
+ resp->id = ioreq->req.id;
+ resp->operation = ioreq->req.operation;
+ resp->status = ioreq->status;
+
blkdev->rings.common.rsp_prod_pvt++;
RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blkdev->rings.common, send_notify);
|