From b11a974dd2d3bb193e3a5cebf77fb8132e389f90 Mon Sep 17 00:00:00 2001 From: Michael Young Date: Feb 06 2014 16:02:37 +0000 Subject: integer overflow in several XSM/Flask hypercalls [XSA-84], Off-by-one error in FLASK_AVC_CACHESTAT hypercall [XSA-85], libvchan failure handling malicious ring indexes [XSA-86] --- diff --git a/xen.spec b/xen.spec index e2093a8..27a02b4 100644 --- a/xen.spec +++ b/xen.spec @@ -46,7 +46,7 @@ Summary: Xen is a virtual machine monitor Name: xen Version: 4.3.1 -Release: 8%{?dist} +Release: 9%{?dist} Group: Development/Libraries License: GPLv2+ and LGPLv2+ and BSD URL: http://xen.org/ @@ -108,6 +108,9 @@ Patch28: xsa77-unstable.patch Patch29: xsa80.patch Patch30: xsa83.patch Patch31: xsa87-unstable-4.3.patch +Patch32: xsa84-unstable-4.3.patch +Patch33: xsa85.patch +Patch34: xsa86.patch Patch100: xen-configure-xend.patch @@ -298,6 +301,9 @@ manage Xen virtual machines. %patch29 -p1 %patch30 -p1 %patch31 -p1 +%patch32 -p1 +%patch33 -p1 +%patch34 -p1 %patch100 -p1 @@ -830,6 +836,11 @@ rm -rf %{buildroot} %endif %changelog +* Thu Feb 06 2014 Michael Young - 4.3.1-9 +- integer overflow in several XSM/Flask hypercalls [XSA-84] +- Off-by-one error in FLASK_AVC_CACHESTAT hypercall [XSA-85] +- libvchan failure handling malicious ring indexes [XSA-86] + * Fri Jan 24 2014 Michael Young - 4.3.1-8 - PHYSDEVOP_{prepare,release}_msix exposed to unprivileged pv guests [XSA-87, CVE-2014-1666] (#1058398) diff --git a/xsa84-unstable-4.3.patch b/xsa84-unstable-4.3.patch new file mode 100644 index 0000000..abff42e --- /dev/null +++ b/xsa84-unstable-4.3.patch @@ -0,0 +1,153 @@ +flask: fix reading strings from guest memory + +Since the string size is being specified by the guest, we must range +check it properly before doing allocations based on it. While for the +two cases that are exposed only to trusted guests (via policy +restriction) this just uses an arbitrary upper limit (PAGE_SIZE), for +the FLASK_[GS]ETBOOL case (which any guest can use) the upper limit +gets enforced based on the longest name across all boolean settings. + +This is XSA-84. + +Reported-by: Matthew Daley +Signed-off-by: Jan Beulich +Acked-by: Daniel De Graaf + +--- a/xen/xsm/flask/flask_op.c ++++ b/xen/xsm/flask/flask_op.c +@@ -53,6 +53,7 @@ static DEFINE_SPINLOCK(sel_sem); + /* global data for booleans */ + static int bool_num = 0; + static int *bool_pending_values = NULL; ++static size_t bool_maxstr; + static int flask_security_make_bools(void); + + extern int ss_initialized; +@@ -71,9 +72,15 @@ static int domain_has_security(struct do + perms, NULL); + } + +-static int flask_copyin_string(XEN_GUEST_HANDLE_PARAM(char) u_buf, char **buf, uint32_t size) ++static int flask_copyin_string(XEN_GUEST_HANDLE_PARAM(char) u_buf, char **buf, ++ size_t size, size_t max_size) + { +- char *tmp = xmalloc_bytes(size + 1); ++ char *tmp; ++ ++ if ( size > max_size ) ++ return -ENOENT; ++ ++ tmp = xmalloc_array(char, size + 1); + if ( !tmp ) + return -ENOMEM; + +@@ -99,7 +106,7 @@ static int flask_security_user(struct xe + if ( rv ) + return rv; + +- rv = flask_copyin_string(arg->u.user, &user, arg->size); ++ rv = flask_copyin_string(arg->u.user, &user, arg->size, PAGE_SIZE); + if ( rv ) + return rv; + +@@ -210,7 +217,7 @@ static int flask_security_context(struct + if ( rv ) + return rv; + +- rv = flask_copyin_string(arg->context, &buf, arg->size); ++ rv = flask_copyin_string(arg->context, &buf, arg->size, PAGE_SIZE); + if ( rv ) + return rv; + +@@ -303,7 +310,7 @@ static int flask_security_resolve_bool(s + if ( arg->bool_id != -1 ) + return 0; + +- rv = flask_copyin_string(arg->name, &name, arg->size); ++ rv = flask_copyin_string(arg->name, &name, arg->size, bool_maxstr); + if ( rv ) + return rv; + +@@ -334,7 +341,7 @@ static int flask_security_set_bool(struc + int num; + int *values; + +- rv = security_get_bools(&num, NULL, &values); ++ rv = security_get_bools(&num, NULL, &values, NULL); + if ( rv != 0 ) + goto out; + +@@ -440,7 +447,7 @@ static int flask_security_make_bools(voi + + xfree(bool_pending_values); + +- ret = security_get_bools(&num, NULL, &values); ++ ret = security_get_bools(&num, NULL, &values, &bool_maxstr); + if ( ret != 0 ) + goto out; + +--- a/xen/xsm/flask/include/conditional.h ++++ b/xen/xsm/flask/include/conditional.h +@@ -13,7 +13,9 @@ + #ifndef _FLASK_CONDITIONAL_H_ + #define _FLASK_CONDITIONAL_H_ + +-int security_get_bools(int *len, char ***names, int **values); ++#include ++ ++int security_get_bools(int *len, char ***names, int **values, size_t *maxstr); + + int security_set_bools(int len, int *values); + +--- a/xen/xsm/flask/ss/services.c ++++ b/xen/xsm/flask/ss/services.c +@@ -1850,7 +1850,7 @@ int security_find_bool(const char *name) + return rv; + } + +-int security_get_bools(int *len, char ***names, int **values) ++int security_get_bools(int *len, char ***names, int **values, size_t *maxstr) + { + int i, rc = -ENOMEM; + +@@ -1858,6 +1858,8 @@ int security_get_bools(int *len, char ** + if ( names ) + *names = NULL; + *values = NULL; ++ if ( maxstr ) ++ *maxstr = 0; + + *len = policydb.p_bools.nprim; + if ( !*len ) +@@ -1879,16 +1881,17 @@ int security_get_bools(int *len, char ** + + for ( i = 0; i < *len; i++ ) + { +- size_t name_len; ++ size_t name_len = strlen(policydb.p_bool_val_to_name[i]); ++ + (*values)[i] = policydb.bool_val_to_struct[i]->state; + if ( names ) { +- name_len = strlen(policydb.p_bool_val_to_name[i]) + 1; +- (*names)[i] = (char*)xmalloc_array(char, name_len); ++ (*names)[i] = xmalloc_array(char, name_len + 1); + if ( !(*names)[i] ) + goto err; +- strlcpy((*names)[i], policydb.p_bool_val_to_name[i], name_len); +- (*names)[i][name_len - 1] = 0; ++ strlcpy((*names)[i], policydb.p_bool_val_to_name[i], name_len + 1); + } ++ if ( maxstr && name_len > *maxstr ) ++ *maxstr = name_len; + } + rc = 0; + out: +@@ -2006,7 +2009,7 @@ static int security_preserve_bools(struc + struct cond_bool_datum *booldatum; + struct cond_node *cur; + +- rc = security_get_bools(&nbools, &bnames, &bvalues); ++ rc = security_get_bools(&nbools, &bnames, &bvalues, NULL); + if ( rc ) + goto out; + for ( i = 0; i < nbools; i++ ) diff --git a/xsa85.patch b/xsa85.patch new file mode 100644 index 0000000..2976b2a --- /dev/null +++ b/xsa85.patch @@ -0,0 +1,31 @@ +From 593bc8c63d582ec0fc2b3a35336106cf9c3a8b34 Mon Sep 17 00:00:00 2001 +From: Matthew Daley +Date: Sun, 12 Jan 2014 14:29:32 +1300 +Subject: [PATCH] xsm/flask: correct off-by-one in + flask_security_avc_cachestats cpu id check + +This is XSA-85 + +Signed-off-by: Matthew Daley +Reviewed-by: Jan Beulich +Reviewed-by: Ian Campbell +--- + xen/xsm/flask/flask_op.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c +index 4426ab9..22878f5 100644 +--- a/xen/xsm/flask/flask_op.c ++++ b/xen/xsm/flask/flask_op.c +@@ -457,7 +457,7 @@ static int flask_security_avc_cachestats(struct xen_flask_cache_stats *arg) + { + struct avc_cache_stats *st; + +- if ( arg->cpu > nr_cpu_ids ) ++ if ( arg->cpu >= nr_cpu_ids ) + return -ENOENT; + if ( !cpu_online(arg->cpu) ) + return -ENOENT; +-- +1.8.5.2 + diff --git a/xsa86.patch b/xsa86.patch new file mode 100644 index 0000000..25ecb1e --- /dev/null +++ b/xsa86.patch @@ -0,0 +1,169 @@ +From b4c452646efd37b4cd0996256dd0ab7bf6ccb7f6 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= + +Date: Mon, 20 Jan 2014 15:51:56 +0000 +Subject: [PATCH] libvchan: Fix handling of invalid ring buffer indices +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The remote (hostile) process can set ring buffer indices to any value +at any time. If that happens, it is possible to get "buffer space" +(either for writing data, or ready for reading) negative or greater +than buffer size. This will end up with buffer overflow in the second +memcpy inside of do_send/do_recv. + +Fix this by introducing new available bytes accessor functions +raw_get_data_ready and raw_get_buffer_space which are robust against +mad ring states, and only return sanitised values. + +Proof sketch of correctness: + +Now {rd,wr}_{cons,prod} are only ever used in the raw available bytes +functions, and in do_send and do_recv. + +The raw available bytes functions do unsigned arithmetic on the +returned values. If the result is "negative" or too big it will be +>ring_size (since we used unsigned arithmetic). Otherwise the result +is a positive in-range value representing a reasonable ring state, in +which case we can safely convert it to int (as the rest of the code +expects). + +do_send and do_recv immediately mask the ring index value with the +ring size. The result is always going to be plausible. If the ring +state has become mad, the worst case is that our behaviour is +inconsistent with the peer's ring pointer. I.e. we read or write to +arguably-incorrect parts of the ring - but always parts of the ring. +And of course if a peer misoperates the ring they can achieve this +effect anyway. + +So the security problem is fixed. + +This is XSA-86. + +(The patch is essentially Ian Jackson's work, although parts of the +commit message are by Marek.) + +Signed-off-by: Marek Marczykowski-Górecki +Signed-off-by: Ian Jackson +Cc: Marek Marczykowski-Górecki +Cc: Joanna Rutkowska +--- + tools/libvchan/io.c | 47 +++++++++++++++++++++++++++++++++++++++++------ + 1 file changed, 41 insertions(+), 6 deletions(-) + +diff --git a/tools/libvchan/io.c b/tools/libvchan/io.c +index 2383364..804c63c 100644 +--- a/tools/libvchan/io.c ++++ b/tools/libvchan/io.c +@@ -111,12 +111,26 @@ static inline int send_notify(struct libxenvchan *ctrl, uint8_t bit) + return 0; + } + ++/* ++ * Get the amount of buffer space available, and do nothing about ++ * notifications. ++ */ ++static inline int raw_get_data_ready(struct libxenvchan *ctrl) ++{ ++ uint32_t ready = rd_prod(ctrl) - rd_cons(ctrl); ++ if (ready >= rd_ring_size(ctrl)) ++ /* We have no way to return errors. Locking up the ring is ++ * better than the alternatives. */ ++ return 0; ++ return ready; ++} ++ + /** + * Get the amount of buffer space available and enable notifications if needed. + */ + static inline int fast_get_data_ready(struct libxenvchan *ctrl, size_t request) + { +- int ready = rd_prod(ctrl) - rd_cons(ctrl); ++ int ready = raw_get_data_ready(ctrl); + if (ready >= request) + return ready; + /* We plan to consume all data; please tell us if you send more */ +@@ -126,7 +140,7 @@ static inline int fast_get_data_ready(struct libxenvchan *ctrl, size_t request) + * will not get notified even though the actual amount of data ready is + * above request. Reread rd_prod to cover this case. + */ +- return rd_prod(ctrl) - rd_cons(ctrl); ++ return raw_get_data_ready(ctrl); + } + + int libxenvchan_data_ready(struct libxenvchan *ctrl) +@@ -135,7 +149,21 @@ int libxenvchan_data_ready(struct libxenvchan *ctrl) + * when it changes + */ + request_notify(ctrl, VCHAN_NOTIFY_WRITE); +- return rd_prod(ctrl) - rd_cons(ctrl); ++ return raw_get_data_ready(ctrl); ++} ++ ++/** ++ * Get the amount of buffer space available, and do nothing ++ * about notifications ++ */ ++static inline int raw_get_buffer_space(struct libxenvchan *ctrl) ++{ ++ uint32_t ready = wr_ring_size(ctrl) - (wr_prod(ctrl) - wr_cons(ctrl)); ++ if (ready > wr_ring_size(ctrl)) ++ /* We have no way to return errors. Locking up the ring is ++ * better than the alternatives. */ ++ return 0; ++ return ready; + } + + /** +@@ -143,7 +171,7 @@ int libxenvchan_data_ready(struct libxenvchan *ctrl) + */ + static inline int fast_get_buffer_space(struct libxenvchan *ctrl, size_t request) + { +- int ready = wr_ring_size(ctrl) - (wr_prod(ctrl) - wr_cons(ctrl)); ++ int ready = raw_get_buffer_space(ctrl); + if (ready >= request) + return ready; + /* We plan to fill the buffer; please tell us when you've read it */ +@@ -153,7 +181,7 @@ static inline int fast_get_buffer_space(struct libxenvchan *ctrl, size_t request + * will not get notified even though the actual amount of buffer space + * is above request. Reread wr_cons to cover this case. + */ +- return wr_ring_size(ctrl) - (wr_prod(ctrl) - wr_cons(ctrl)); ++ return raw_get_buffer_space(ctrl); + } + + int libxenvchan_buffer_space(struct libxenvchan *ctrl) +@@ -162,7 +190,7 @@ int libxenvchan_buffer_space(struct libxenvchan *ctrl) + * when it changes + */ + request_notify(ctrl, VCHAN_NOTIFY_READ); +- return wr_ring_size(ctrl) - (wr_prod(ctrl) - wr_cons(ctrl)); ++ return raw_get_buffer_space(ctrl); + } + + int libxenvchan_wait(struct libxenvchan *ctrl) +@@ -176,6 +204,8 @@ int libxenvchan_wait(struct libxenvchan *ctrl) + + /** + * returns -1 on error, or size on success ++ * ++ * caller must have checked that enough space is available + */ + static int do_send(struct libxenvchan *ctrl, const void *data, size_t size) + { +@@ -248,6 +278,11 @@ int libxenvchan_write(struct libxenvchan *ctrl, const void *data, size_t size) + } + } + ++/** ++ * returns -1 on error, or size on success ++ * ++ * caller must have checked that enough data is available ++ */ + static int do_recv(struct libxenvchan *ctrl, void *data, size_t size) + { + int real_idx = rd_cons(ctrl) & (rd_ring_size(ctrl) - 1); +-- +1.7.10.4 +