Xen Security Advisory 84 (CVE-2014-1891, CVE-2014-1892, CVE-2014-1893, CVE-2014-1894) - integer overflow in several XSM/Flask hypercalls

References: bnc#860163 XSA-84

flask: restrict allocations done by hypercall interface

Other than in 4.2 and newer, we're not having an overflow issue here,
but uncontrolled exposure of the operations opens the host to be driven
out of memory by an arbitrary guest. Since all operations other than
FLASK_LOAD simply deal with ASCII strings, limiting the allocations
(and incoming buffer sizes) to a page worth of memory seems like the
best thing we can do.

Consequently, in order to not expose the larger allocation to arbitrary
guests, the permission check for FLASK_LOAD needs to be pulled ahead of
the allocation (and it's perhaps worth noting that - afaict - it was
pointlessly done with the sel_sem spin lock held).

Note that this breaks FLASK_AVC_CACHESTATS on systems with sufficiently
many CPUs (as requiring a buffer bigger than PAGE_SIZE there). No
attempt is made to address this here, as it would needlessly complicate
this fix with rather little gain.

This is XSA-84.

Reported-by: Matthew Daley <[email protected]>
Signed-off-by: Jan Beulich <[email protected]>

The index of boolean variables in FLASK_{GET,SET}BOOL was not always
checked against the bounds of the array.

Reported-by: John McDermott <[email protected]>
Signed-off-by: Daniel De Graaf <[email protected]>

--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -573,7 +573,7 @@ static int flask_security_setavc_thresho
 static int flask_security_set_bool(char *buf, uint32_t count)
 {
     int length = -EFAULT;
-    int i, new_value;
+    unsigned int i, new_value;
 
     spin_lock(&sel_sem);
 
@@ -585,6 +585,9 @@ static int flask_security_set_bool(char 
     if ( sscanf(buf, "%d %d", &i, &new_value) != 2 )
         goto out;
 
+    if ( i >= bool_num )
+        goto out;
+
     if ( new_value )
     {
         new_value = 1;
@@ -734,10 +737,6 @@ static int flask_security_load(char *buf
 
     spin_lock(&sel_sem);
 
-    length = domain_has_security(current->domain, SECURITY__LOAD_POLICY);
-    if ( length )
-        goto out;
-
     length = security_load_policy(buf, count);
     if ( length )
         goto out;
@@ -853,7 +852,15 @@ long do_flask_op(XEN_GUEST_HANDLE(xsm_op
     if ( op->cmd > FLASK_LAST)
         return -EINVAL;
 
-    if ( op->size > MAX_POLICY_SIZE )
+    if ( op->cmd == FLASK_LOAD )
+    {
+        rc = domain_has_security(current->domain, SECURITY__LOAD_POLICY);
+        if ( rc )
+            return rc;
+        if ( op->size > MAX_POLICY_SIZE )
+            return -EINVAL;
+    }
+    else if ( op->size >= PAGE_SIZE )
         return -EINVAL;
 
     if ( (op->buf == NULL && op->size != 0) || 
--- a/xen/xsm/flask/ss/services.c
+++ b/xen/xsm/flask/ss/services.c
@@ -1991,7 +1991,7 @@ int security_get_bool_value(int bool)
     POLICY_RDLOCK;
 
     len = policydb.p_bools.nprim;
-    if ( bool >= len )
+    if ( bool >= len || bool < 0 )
     {
         rc = -EFAULT;
         goto out;

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 <[email protected]>
Signed-off-by: Jan Beulich <[email protected]>
Acked-by: Daniel De Graaf <[email protected]>

--- 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(char) u_buf, char **buf, uint32_t size)
+static int flask_copyin_string(XEN_GUEST_HANDLE(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 <xen/types.h>
+
+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
@@ -1900,7 +1900,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;
 
@@ -1908,6 +1908,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 )
@@ -1929,16 +1931,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:
@@ -2056,7 +2059,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++ )

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 <[email protected]>
Signed-off-by: Jan Beulich <[email protected]>
Acked-by: Daniel De Graaf <[email protected]>

--- 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 <xen/types.h>
+
+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++ )