xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] flask: XSA-84 follow-ups
@ 2014-02-07  9:41 Jan Beulich
  2014-02-07  9:46 ` [PATCH 1/4] flask: fix memory leaks Jan Beulich
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Jan Beulich @ 2014-02-07  9:41 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, dgdegra

1: fix memory leaks
2: fix error propagation from flask_security_set_bool()
3: check permissions first thing in flask_security_set_bool()
4: add compat mode guest support

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Release-wise, I would think that 1-3 should certainly go in. While I'd
like 4 to be in for 4.4 too, I realize that's a little more intrusive than
one would want at this point.

Jan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] flask: fix memory leaks
  2014-02-07  9:41 [PATCH 0/4] flask: XSA-84 follow-ups Jan Beulich
@ 2014-02-07  9:46 ` Jan Beulich
  2014-02-07 12:51   ` Andrew Cooper
  2014-02-07  9:47 ` [PATCH 2/4] flask: fix error propagation from flask_security_set_bool() Jan Beulich
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2014-02-07  9:46 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, dgdegra

[-- Attachment #1: Type: text/plain, Size: 998 bytes --]

Plus, in the case of security_preserve_bools(), prevent double freeing
in the case of security_get_bools() failing.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -347,6 +347,7 @@ static int flask_security_set_bool(struc
 
         if ( arg->bool_id >= num )
         {
+            xfree(values);
             rv = -ENOENT;
             goto out;
         }
--- a/xen/xsm/flask/ss/services.c
+++ b/xen/xsm/flask/ss/services.c
@@ -1902,6 +1902,7 @@ err:
     {
         for ( i = 0; i < *len; i++ )
             xfree((*names)[i]);
+        xfree(*names);
     }
     xfree(*values);
     goto out;
@@ -2011,7 +2012,7 @@ static int security_preserve_bools(struc
 
     rc = security_get_bools(&nbools, &bnames, &bvalues, NULL);
     if ( rc )
-        goto out;
+        return rc;
     for ( i = 0; i < nbools; i++ )
     {
         booldatum = hashtab_search(p->p_bools.table, bnames[i]);




[-- Attachment #2: flask-leaks.patch --]
[-- Type: text/plain, Size: 1019 bytes --]

flask: fix memory leaks

Plus, in the case of security_preserve_bools(), prevent double freeing
in the case of security_get_bools() failing.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -347,6 +347,7 @@ static int flask_security_set_bool(struc
 
         if ( arg->bool_id >= num )
         {
+            xfree(values);
             rv = -ENOENT;
             goto out;
         }
--- a/xen/xsm/flask/ss/services.c
+++ b/xen/xsm/flask/ss/services.c
@@ -1902,6 +1902,7 @@ err:
     {
         for ( i = 0; i < *len; i++ )
             xfree((*names)[i]);
+        xfree(*names);
     }
     xfree(*values);
     goto out;
@@ -2011,7 +2012,7 @@ static int security_preserve_bools(struc
 
     rc = security_get_bools(&nbools, &bnames, &bvalues, NULL);
     if ( rc )
-        goto out;
+        return rc;
     for ( i = 0; i < nbools; i++ )
     {
         booldatum = hashtab_search(p->p_bools.table, bnames[i]);

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/4] flask: fix error propagation from flask_security_set_bool()
  2014-02-07  9:41 [PATCH 0/4] flask: XSA-84 follow-ups Jan Beulich
  2014-02-07  9:46 ` [PATCH 1/4] flask: fix memory leaks Jan Beulich
@ 2014-02-07  9:47 ` Jan Beulich
  2014-02-07 12:56   ` Andrew Cooper
  2014-02-07  9:47 ` [PATCH 3/4] flask: check permissions first thing in flask_security_set_bool() Jan Beulich
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2014-02-07  9:47 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, dgdegra

[-- Attachment #1: Type: text/plain, Size: 685 bytes --]

The function should return an error when flask_security_make_bools() as
well as when the input ID is out of range.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -364,9 +364,10 @@ static int flask_security_set_bool(struc
     else
     {
         if ( !bool_pending_values )
-            flask_security_make_bools();
-
-        if ( arg->bool_id >= bool_num )
+            rv = flask_security_make_bools();
+        if ( !rv && arg->bool_id >= bool_num )
+            rv = -ENOENT;
+        if ( rv )
             goto out;
 
         bool_pending_values[arg->bool_id] = !!(arg->new_value);




[-- Attachment #2: flask-set-bool-err.patch --]
[-- Type: text/plain, Size: 742 bytes --]

flask: fix error propagation from flask_security_set_bool()

The function should return an error when flask_security_make_bools() as
well as when the input ID is out of range.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -364,9 +364,10 @@ static int flask_security_set_bool(struc
     else
     {
         if ( !bool_pending_values )
-            flask_security_make_bools();
-
-        if ( arg->bool_id >= bool_num )
+            rv = flask_security_make_bools();
+        if ( !rv && arg->bool_id >= bool_num )
+            rv = -ENOENT;
+        if ( rv )
             goto out;
 
         bool_pending_values[arg->bool_id] = !!(arg->new_value);

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/4] flask: check permissions first thing in flask_security_set_bool()
  2014-02-07  9:41 [PATCH 0/4] flask: XSA-84 follow-ups Jan Beulich
  2014-02-07  9:46 ` [PATCH 1/4] flask: fix memory leaks Jan Beulich
  2014-02-07  9:47 ` [PATCH 2/4] flask: fix error propagation from flask_security_set_bool() Jan Beulich
@ 2014-02-07  9:47 ` Jan Beulich
  2014-02-07 12:57   ` Andrew Cooper
  2014-02-07  9:48 ` [PATCH 4/4] flask: add compat mode guest support Jan Beulich
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2014-02-07  9:47 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, dgdegra

[-- Attachment #1: Type: text/plain, Size: 596 bytes --]

Nothing else should be done if the caller isn't permitted to set
boolean values.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -326,11 +326,11 @@ static int flask_security_set_bool(struc
 {
     int rv;
 
-    rv = flask_security_resolve_bool(arg);
+    rv = domain_has_security(current->domain, SECURITY__SETBOOL);
     if ( rv )
         return rv;
 
-    rv = domain_has_security(current->domain, SECURITY__SETBOOL);
+    rv = flask_security_resolve_bool(arg);
     if ( rv )
         return rv;
 




[-- Attachment #2: flask-set-bool-perm-first.patch --]
[-- Type: text/plain, Size: 659 bytes --]

flask: check permissions first thing in flask_security_set_bool()

Nothing else should be done if the caller isn't permitted to set
boolean values.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -326,11 +326,11 @@ static int flask_security_set_bool(struc
 {
     int rv;
 
-    rv = flask_security_resolve_bool(arg);
+    rv = domain_has_security(current->domain, SECURITY__SETBOOL);
     if ( rv )
         return rv;
 
-    rv = domain_has_security(current->domain, SECURITY__SETBOOL);
+    rv = flask_security_resolve_bool(arg);
     if ( rv )
         return rv;
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 4/4] flask: add compat mode guest support
  2014-02-07  9:41 [PATCH 0/4] flask: XSA-84 follow-ups Jan Beulich
                   ` (2 preceding siblings ...)
  2014-02-07  9:47 ` [PATCH 3/4] flask: check permissions first thing in flask_security_set_bool() Jan Beulich
@ 2014-02-07  9:48 ` Jan Beulich
  2014-02-07 11:29 ` [PATCH 0/4] flask: XSA-84 follow-ups George Dunlap
  2014-02-10 20:22 ` Daniel De Graaf
  5 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2014-02-07  9:48 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, dgdegra, Keir Fraser

[-- Attachment #1: Type: text/plain, Size: 12028 bytes --]

... which has been missing since the introduction of the new interface
in the 4.2 development cycle.

In the course of this I also noticed that the compat header generation
failed to make use of move-if-changed.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -404,7 +404,7 @@ ENTRY(compat_hypercall_table)
         .quad compat_vcpu_op
         .quad compat_ni_hypercall       /* 25 */
         .quad compat_mmuext_op
-        .quad do_xsm_op
+        .quad compat_xsm_op
         .quad compat_nmi_op
         .quad compat_sched_op
         .quad compat_callback_op        /* 30 */
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -27,6 +27,7 @@ headers-$(CONFIG_X86)     += compat/arch
 headers-$(CONFIG_X86)     += compat/arch-x86/xen.h
 headers-$(CONFIG_X86)     += compat/arch-x86/xen-$(compat-arch-y).h
 headers-y                 += compat/arch-$(compat-arch-y).h compat/xlat.h
+headers-$(FLASK_ENABLE)   += compat/xsm/flask_op.h
 
 cppflags-y                := -include public/xen-compat.h
 cppflags-$(CONFIG_X86)    += -m32
@@ -54,7 +55,7 @@ compat/%.h: compat/%.i Makefile $(BASEDI
 	$(PYTHON) $(BASEDIR)/tools/compat-build-header.py | uniq >>$@.new; \
 	$(if $(suffix-y),echo "$(suffix-y)" >>$@.new;) \
 	echo "#endif /* $$id */" >>$@.new
-	mv -f $@.new $@
+	$(call move-if-changed,$@.new,$@)
 
 compat/%.i: compat/%.c Makefile
 	$(CPP) $(filter-out -M% .%.d -include %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
@@ -63,15 +64,17 @@ compat/%.c: public/%.h xlat.lst Makefile
 	mkdir -p $(@D)
 	grep -v 'DEFINE_XEN_GUEST_HANDLE(long)' $< | \
 	$(PYTHON) $(BASEDIR)/tools/compat-build-source.py >$@.new
-	mv -f $@.new $@
+	$(call move-if-changed,$@.new,$@)
 
 compat/xlat.h: xlat.lst $(filter-out compat/xlat.h,$(headers-y)) $(BASEDIR)/tools/get-fields.sh Makefile
 	export PYTHON=$(PYTHON); \
 	grep -v '^[	 ]*#' xlat.lst | \
 	while read what name hdr; do \
-		$(SHELL) $(BASEDIR)/tools/get-fields.sh "$$what" compat_$$name $$(echo compat/$$hdr | sed 's,@arch@,$(compat-arch-y),g') || exit $$?; \
+		hdr="compat/$$(echo $$hdr | sed 's,@arch@,$(compat-arch-y),g')"; \
+		echo '$(headers-y)' | grep -q "$$hdr" || continue; \
+		$(SHELL) $(BASEDIR)/tools/get-fields.sh "$$what" compat_$$name $$hdr || exit $$?; \
 	done >$@.new
-	mv -f $@.new $@
+	$(call move-if-changed,$@.new,$@)
 
 ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
 
@@ -79,7 +82,7 @@ all: headers.chk
 
 headers.chk: $(filter-out public/arch-% public/%ctl.h public/xsm/% public/%hvm/save.h, $(wildcard public/*.h public/*/*.h) $(public-y)) Makefile
 	for i in $(filter %.h,$^); do $(CC) -ansi -include stdint.h -Wall -W -Werror -S -o /dev/null -xc $$i || exit 1; echo $$i; done >$@.new
-	mv $@.new $@
+	$(call move-if-changed,$@.new,$@)
 
 endif
 
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -99,3 +99,16 @@
 !	vcpu_set_singleshot_timer	vcpu.h
 ?	xenoprof_init			xenoprof.h
 ?	xenoprof_passive		xenoprof.h
+?	flask_access			xsm/flask_op.h
+!	flask_boolean			xsm/flask_op.h
+?	flask_cache_stats		xsm/flask_op.h
+?	flask_hash_stats		xsm/flask_op.h
+!	flask_load			xsm/flask_op.h
+?	flask_ocontext			xsm/flask_op.h
+?	flask_peersid			xsm/flask_op.h
+?	flask_relabel			xsm/flask_op.h
+?	flask_setavc_threshold		xsm/flask_op.h
+?	flask_setenforce		xsm/flask_op.h
+!	flask_sid_context		xsm/flask_op.h
+?	flask_transition		xsm/flask_op.h
+!	flask_userlist			xsm/flask_op.h
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -412,6 +412,13 @@ static XSM_INLINE long xsm_do_xsm_op(XEN
     return -ENOSYS;
 }
 
+#ifdef CONFIG_COMPAT
+static XSM_INLINE int xsm_do_compat_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
+{
+    return -ENOSYS;
+}
+#endif
+
 static XSM_INLINE char *xsm_show_irq_sid(int irq)
 {
     return NULL;
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -129,6 +129,9 @@ struct xsm_operations {
     int (*tmem_control)(void);
 
     long (*do_xsm_op) (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op);
+#ifdef CONFIG_COMPAT
+    int (*do_compat_op) (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op);
+#endif
 
     int (*hvm_param) (struct domain *d, unsigned long op);
     int (*hvm_param_nested) (struct domain *d);
@@ -499,6 +502,13 @@ static inline long xsm_do_xsm_op (XEN_GU
     return xsm_ops->do_xsm_op(op);
 }
 
+#ifdef CONFIG_COMPAT
+static inline int xsm_do_compat_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
+{
+    return xsm_ops->do_compat_op(op);
+}
+#endif
+
 static inline int xsm_hvm_param (xsm_default_t def, struct domain *d, unsigned long op)
 {
     return xsm_ops->hvm_param(d, op);
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -105,6 +105,9 @@ void xsm_fixup_ops (struct xsm_operation
     set_to_dummy_if_null(ops, hvm_param_nested);
 
     set_to_dummy_if_null(ops, do_xsm_op);
+#ifdef CONFIG_COMPAT
+    set_to_dummy_if_null(ops, do_compat_op);
+#endif
 
     set_to_dummy_if_null(ops, add_to_physmap);
     set_to_dummy_if_null(ops, remove_from_physmap);
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -7,7 +7,7 @@
  *  it under the terms of the GNU General Public License version 2,
  *  as published by the Free Software Foundation.
  */
-
+#ifndef COMPAT
 #include <xen/errno.h>
 #include <xen/event.h>
 #include <xsm/xsm.h>
@@ -20,6 +20,10 @@
 #include <objsec.h>
 #include <conditional.h>
 
+#define ret_t long
+#define _copy_to_guest copy_to_guest
+#define _copy_from_guest copy_from_guest
+
 #ifdef FLASK_DEVELOP
 int flask_enforcing = 0;
 integer_param("flask_enforcing", flask_enforcing);
@@ -95,6 +99,8 @@ static int flask_copyin_string(XEN_GUEST
     return 0;
 }
 
+#endif /* COMPAT */
+
 static int flask_security_user(struct xen_flask_userlist *arg)
 {
     char *user;
@@ -119,7 +125,7 @@ static int flask_security_user(struct xe
 
     arg->size = nsids;
 
-    if ( copy_to_guest(arg->u.sids, sids, nsids) )
+    if ( _copy_to_guest(arg->u.sids, sids, nsids) )
         rv = -EFAULT;
 
     xfree(sids);
@@ -128,6 +134,8 @@ static int flask_security_user(struct xe
     return rv;
 }
 
+#ifndef COMPAT
+
 static int flask_security_relabel(struct xen_flask_transition *arg)
 {
     int rv;
@@ -208,6 +216,8 @@ static int flask_security_setenforce(str
     return 0;
 }
 
+#endif /* COMPAT */
+
 static int flask_security_context(struct xen_flask_sid_context *arg)
 {
     int rv;
@@ -252,7 +262,7 @@ static int flask_security_sid(struct xen
 
     arg->size = len;
 
-    if ( !rv && copy_to_guest(arg->context, context, len) )
+    if ( !rv && _copy_to_guest(arg->context, context, len) )
         rv = -EFAULT;
 
     xfree(context);
@@ -260,6 +270,8 @@ static int flask_security_sid(struct xen
     return rv;
 }
 
+#ifndef COMPAT
+
 int flask_disable(void)
 {
     static int flask_disabled = 0;
@@ -302,6 +314,8 @@ static int flask_security_setavc_thresho
     return rv;
 }
 
+#endif /* COMPAT */
+
 static int flask_security_resolve_bool(struct xen_flask_boolean *arg)
 {
     char *name;
@@ -382,24 +396,6 @@ static int flask_security_set_bool(struc
     return rv;
 }
 
-static int flask_security_commit_bools(void)
-{
-    int rv;
-
-    spin_lock(&sel_sem);
-
-    rv = domain_has_security(current->domain, SECURITY__SETBOOL);
-    if ( rv )
-        goto out;
-
-    if ( bool_pending_values )
-        rv = security_set_bools(bool_num, bool_pending_values);
-    
- out:
-    spin_unlock(&sel_sem);
-    return rv;
-}
-
 static int flask_security_get_bool(struct xen_flask_boolean *arg)
 {
     int rv;
@@ -431,7 +427,7 @@ static int flask_security_get_bool(struc
             rv = -ERANGE;
         arg->size = nameout_len;
  
-        if ( !rv && copy_to_guest(arg->name, nameout, nameout_len) )
+        if ( !rv && _copy_to_guest(arg->name, nameout, nameout_len) )
             rv = -EFAULT;
         xfree(nameout);
     }
@@ -441,6 +437,26 @@ static int flask_security_get_bool(struc
     return rv;
 }
 
+#ifndef COMPAT
+
+static int flask_security_commit_bools(void)
+{
+    int rv;
+
+    spin_lock(&sel_sem);
+
+    rv = domain_has_security(current->domain, SECURITY__SETBOOL);
+    if ( rv )
+        goto out;
+
+    if ( bool_pending_values )
+        rv = security_set_bools(bool_num, bool_pending_values);
+
+ out:
+    spin_unlock(&sel_sem);
+    return rv;
+}
+
 static int flask_security_make_bools(void)
 {
     int ret = 0;
@@ -484,6 +500,7 @@ static int flask_security_avc_cachestats
 }
 
 #endif
+#endif /* COMPAT */
 
 static int flask_security_load(struct xen_flask_load *load)
 {
@@ -501,7 +518,7 @@ static int flask_security_load(struct xe
     if ( !buf )
         return -ENOMEM;
 
-    if ( copy_from_guest(buf, load->buffer, load->size) )
+    if ( _copy_from_guest(buf, load->buffer, load->size) )
     {
         ret = -EFAULT;
         goto out_free;
@@ -524,6 +541,8 @@ static int flask_security_load(struct xe
     return ret;
 }
 
+#ifndef COMPAT
+
 static int flask_ocontext_del(struct xen_flask_ocontext *arg)
 {
     int rv;
@@ -636,7 +655,9 @@ static int flask_relabel_domain(struct x
     return rc;
 }
 
-long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op)
+#endif /* !COMPAT */
+
+ret_t do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op)
 {
     xen_flask_op_t op;
     int rv;
@@ -763,3 +784,52 @@ long do_flask_op(XEN_GUEST_HANDLE_PARAM(
  out:
     return rv;
 }
+
+#ifndef COMPAT
+#undef _copy_to_guest
+#define _copy_to_guest copy_to_compat
+#undef _copy_from_guest
+#define _copy_from_guest copy_from_compat
+
+#include <compat/event_channel.h>
+#include <compat/xsm/flask_op.h>
+
+CHECK_flask_access;
+CHECK_flask_cache_stats;
+CHECK_flask_hash_stats;
+CHECK_flask_ocontext;
+CHECK_flask_peersid;
+CHECK_flask_relabel;
+CHECK_flask_setavc_threshold;
+CHECK_flask_setenforce;
+CHECK_flask_transition;
+
+#define COMPAT
+#define flask_copyin_string(ch, pb, sz, mx) ({ \
+	XEN_GUEST_HANDLE_PARAM(char) gh; \
+	guest_from_compat_handle(gh, ch); \
+	flask_copyin_string(gh, pb, sz, mx); \
+})
+
+#define xen_flask_load compat_flask_load
+#define flask_security_load compat_security_load
+
+#define xen_flask_userlist compat_flask_userlist
+#define flask_security_user compat_security_user
+
+#define xen_flask_sid_context compat_flask_sid_context
+#define flask_security_context compat_security_context
+#define flask_security_sid compat_security_sid
+
+#define xen_flask_boolean compat_flask_boolean
+#define flask_security_resolve_bool compat_security_resolve_bool
+#define flask_security_get_bool compat_security_get_bool
+#define flask_security_set_bool compat_security_set_bool
+
+#define xen_flask_op_t compat_flask_op_t
+#undef ret_t
+#define ret_t int
+#define do_flask_op compat_flask_op
+
+#include "flask_op.c"
+#endif
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1461,6 +1461,7 @@ static int flask_map_gmfn_foreign(struct
 #endif
 
 long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
+int compat_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
 
 static struct xsm_operations flask_ops = {
     .security_domaininfo = flask_security_domaininfo,
@@ -1535,6 +1536,9 @@ static struct xsm_operations flask_ops =
     .hvm_param_nested = flask_hvm_param_nested,
 
     .do_xsm_op = do_flask_op,
+#ifdef CONFIG_COMPAT
+    .do_compat_op = compat_flask_op,
+#endif
 
     .add_to_physmap = flask_add_to_physmap,
     .remove_from_physmap = flask_remove_from_physmap,
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -116,4 +116,9 @@ long do_xsm_op (XEN_GUEST_HANDLE_PARAM(x
     return xsm_do_xsm_op(op);
 }
 
-
+#ifdef CONFIG_COMPAT
+int compat_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
+{
+    return xsm_do_compat_op(op);
+}
+#endif



[-- Attachment #2: flask-op-compat.patch --]
[-- Type: text/plain, Size: 12064 bytes --]

flask: add compat mode guest support

... which has been missing since the introduction of the new interface
in the 4.2 development cycle.

In the course of this I also noticed that the compat header generation
failed to make use of move-if-changed.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -404,7 +404,7 @@ ENTRY(compat_hypercall_table)
         .quad compat_vcpu_op
         .quad compat_ni_hypercall       /* 25 */
         .quad compat_mmuext_op
-        .quad do_xsm_op
+        .quad compat_xsm_op
         .quad compat_nmi_op
         .quad compat_sched_op
         .quad compat_callback_op        /* 30 */
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -27,6 +27,7 @@ headers-$(CONFIG_X86)     += compat/arch
 headers-$(CONFIG_X86)     += compat/arch-x86/xen.h
 headers-$(CONFIG_X86)     += compat/arch-x86/xen-$(compat-arch-y).h
 headers-y                 += compat/arch-$(compat-arch-y).h compat/xlat.h
+headers-$(FLASK_ENABLE)   += compat/xsm/flask_op.h
 
 cppflags-y                := -include public/xen-compat.h
 cppflags-$(CONFIG_X86)    += -m32
@@ -54,7 +55,7 @@ compat/%.h: compat/%.i Makefile $(BASEDI
 	$(PYTHON) $(BASEDIR)/tools/compat-build-header.py | uniq >>$@.new; \
 	$(if $(suffix-y),echo "$(suffix-y)" >>$@.new;) \
 	echo "#endif /* $$id */" >>$@.new
-	mv -f $@.new $@
+	$(call move-if-changed,$@.new,$@)
 
 compat/%.i: compat/%.c Makefile
 	$(CPP) $(filter-out -M% .%.d -include %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
@@ -63,15 +64,17 @@ compat/%.c: public/%.h xlat.lst Makefile
 	mkdir -p $(@D)
 	grep -v 'DEFINE_XEN_GUEST_HANDLE(long)' $< | \
 	$(PYTHON) $(BASEDIR)/tools/compat-build-source.py >$@.new
-	mv -f $@.new $@
+	$(call move-if-changed,$@.new,$@)
 
 compat/xlat.h: xlat.lst $(filter-out compat/xlat.h,$(headers-y)) $(BASEDIR)/tools/get-fields.sh Makefile
 	export PYTHON=$(PYTHON); \
 	grep -v '^[	 ]*#' xlat.lst | \
 	while read what name hdr; do \
-		$(SHELL) $(BASEDIR)/tools/get-fields.sh "$$what" compat_$$name $$(echo compat/$$hdr | sed 's,@arch@,$(compat-arch-y),g') || exit $$?; \
+		hdr="compat/$$(echo $$hdr | sed 's,@arch@,$(compat-arch-y),g')"; \
+		echo '$(headers-y)' | grep -q "$$hdr" || continue; \
+		$(SHELL) $(BASEDIR)/tools/get-fields.sh "$$what" compat_$$name $$hdr || exit $$?; \
 	done >$@.new
-	mv -f $@.new $@
+	$(call move-if-changed,$@.new,$@)
 
 ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
 
@@ -79,7 +82,7 @@ all: headers.chk
 
 headers.chk: $(filter-out public/arch-% public/%ctl.h public/xsm/% public/%hvm/save.h, $(wildcard public/*.h public/*/*.h) $(public-y)) Makefile
 	for i in $(filter %.h,$^); do $(CC) -ansi -include stdint.h -Wall -W -Werror -S -o /dev/null -xc $$i || exit 1; echo $$i; done >$@.new
-	mv $@.new $@
+	$(call move-if-changed,$@.new,$@)
 
 endif
 
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -99,3 +99,16 @@
 !	vcpu_set_singleshot_timer	vcpu.h
 ?	xenoprof_init			xenoprof.h
 ?	xenoprof_passive		xenoprof.h
+?	flask_access			xsm/flask_op.h
+!	flask_boolean			xsm/flask_op.h
+?	flask_cache_stats		xsm/flask_op.h
+?	flask_hash_stats		xsm/flask_op.h
+!	flask_load			xsm/flask_op.h
+?	flask_ocontext			xsm/flask_op.h
+?	flask_peersid			xsm/flask_op.h
+?	flask_relabel			xsm/flask_op.h
+?	flask_setavc_threshold		xsm/flask_op.h
+?	flask_setenforce		xsm/flask_op.h
+!	flask_sid_context		xsm/flask_op.h
+?	flask_transition		xsm/flask_op.h
+!	flask_userlist			xsm/flask_op.h
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -412,6 +412,13 @@ static XSM_INLINE long xsm_do_xsm_op(XEN
     return -ENOSYS;
 }
 
+#ifdef CONFIG_COMPAT
+static XSM_INLINE int xsm_do_compat_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
+{
+    return -ENOSYS;
+}
+#endif
+
 static XSM_INLINE char *xsm_show_irq_sid(int irq)
 {
     return NULL;
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -129,6 +129,9 @@ struct xsm_operations {
     int (*tmem_control)(void);
 
     long (*do_xsm_op) (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op);
+#ifdef CONFIG_COMPAT
+    int (*do_compat_op) (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op);
+#endif
 
     int (*hvm_param) (struct domain *d, unsigned long op);
     int (*hvm_param_nested) (struct domain *d);
@@ -499,6 +502,13 @@ static inline long xsm_do_xsm_op (XEN_GU
     return xsm_ops->do_xsm_op(op);
 }
 
+#ifdef CONFIG_COMPAT
+static inline int xsm_do_compat_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
+{
+    return xsm_ops->do_compat_op(op);
+}
+#endif
+
 static inline int xsm_hvm_param (xsm_default_t def, struct domain *d, unsigned long op)
 {
     return xsm_ops->hvm_param(d, op);
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -105,6 +105,9 @@ void xsm_fixup_ops (struct xsm_operation
     set_to_dummy_if_null(ops, hvm_param_nested);
 
     set_to_dummy_if_null(ops, do_xsm_op);
+#ifdef CONFIG_COMPAT
+    set_to_dummy_if_null(ops, do_compat_op);
+#endif
 
     set_to_dummy_if_null(ops, add_to_physmap);
     set_to_dummy_if_null(ops, remove_from_physmap);
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -7,7 +7,7 @@
  *  it under the terms of the GNU General Public License version 2,
  *  as published by the Free Software Foundation.
  */
-
+#ifndef COMPAT
 #include <xen/errno.h>
 #include <xen/event.h>
 #include <xsm/xsm.h>
@@ -20,6 +20,10 @@
 #include <objsec.h>
 #include <conditional.h>
 
+#define ret_t long
+#define _copy_to_guest copy_to_guest
+#define _copy_from_guest copy_from_guest
+
 #ifdef FLASK_DEVELOP
 int flask_enforcing = 0;
 integer_param("flask_enforcing", flask_enforcing);
@@ -95,6 +99,8 @@ static int flask_copyin_string(XEN_GUEST
     return 0;
 }
 
+#endif /* COMPAT */
+
 static int flask_security_user(struct xen_flask_userlist *arg)
 {
     char *user;
@@ -119,7 +125,7 @@ static int flask_security_user(struct xe
 
     arg->size = nsids;
 
-    if ( copy_to_guest(arg->u.sids, sids, nsids) )
+    if ( _copy_to_guest(arg->u.sids, sids, nsids) )
         rv = -EFAULT;
 
     xfree(sids);
@@ -128,6 +134,8 @@ static int flask_security_user(struct xe
     return rv;
 }
 
+#ifndef COMPAT
+
 static int flask_security_relabel(struct xen_flask_transition *arg)
 {
     int rv;
@@ -208,6 +216,8 @@ static int flask_security_setenforce(str
     return 0;
 }
 
+#endif /* COMPAT */
+
 static int flask_security_context(struct xen_flask_sid_context *arg)
 {
     int rv;
@@ -252,7 +262,7 @@ static int flask_security_sid(struct xen
 
     arg->size = len;
 
-    if ( !rv && copy_to_guest(arg->context, context, len) )
+    if ( !rv && _copy_to_guest(arg->context, context, len) )
         rv = -EFAULT;
 
     xfree(context);
@@ -260,6 +270,8 @@ static int flask_security_sid(struct xen
     return rv;
 }
 
+#ifndef COMPAT
+
 int flask_disable(void)
 {
     static int flask_disabled = 0;
@@ -302,6 +314,8 @@ static int flask_security_setavc_thresho
     return rv;
 }
 
+#endif /* COMPAT */
+
 static int flask_security_resolve_bool(struct xen_flask_boolean *arg)
 {
     char *name;
@@ -382,24 +396,6 @@ static int flask_security_set_bool(struc
     return rv;
 }
 
-static int flask_security_commit_bools(void)
-{
-    int rv;
-
-    spin_lock(&sel_sem);
-
-    rv = domain_has_security(current->domain, SECURITY__SETBOOL);
-    if ( rv )
-        goto out;
-
-    if ( bool_pending_values )
-        rv = security_set_bools(bool_num, bool_pending_values);
-    
- out:
-    spin_unlock(&sel_sem);
-    return rv;
-}
-
 static int flask_security_get_bool(struct xen_flask_boolean *arg)
 {
     int rv;
@@ -431,7 +427,7 @@ static int flask_security_get_bool(struc
             rv = -ERANGE;
         arg->size = nameout_len;
  
-        if ( !rv && copy_to_guest(arg->name, nameout, nameout_len) )
+        if ( !rv && _copy_to_guest(arg->name, nameout, nameout_len) )
             rv = -EFAULT;
         xfree(nameout);
     }
@@ -441,6 +437,26 @@ static int flask_security_get_bool(struc
     return rv;
 }
 
+#ifndef COMPAT
+
+static int flask_security_commit_bools(void)
+{
+    int rv;
+
+    spin_lock(&sel_sem);
+
+    rv = domain_has_security(current->domain, SECURITY__SETBOOL);
+    if ( rv )
+        goto out;
+
+    if ( bool_pending_values )
+        rv = security_set_bools(bool_num, bool_pending_values);
+
+ out:
+    spin_unlock(&sel_sem);
+    return rv;
+}
+
 static int flask_security_make_bools(void)
 {
     int ret = 0;
@@ -484,6 +500,7 @@ static int flask_security_avc_cachestats
 }
 
 #endif
+#endif /* COMPAT */
 
 static int flask_security_load(struct xen_flask_load *load)
 {
@@ -501,7 +518,7 @@ static int flask_security_load(struct xe
     if ( !buf )
         return -ENOMEM;
 
-    if ( copy_from_guest(buf, load->buffer, load->size) )
+    if ( _copy_from_guest(buf, load->buffer, load->size) )
     {
         ret = -EFAULT;
         goto out_free;
@@ -524,6 +541,8 @@ static int flask_security_load(struct xe
     return ret;
 }
 
+#ifndef COMPAT
+
 static int flask_ocontext_del(struct xen_flask_ocontext *arg)
 {
     int rv;
@@ -636,7 +655,9 @@ static int flask_relabel_domain(struct x
     return rc;
 }
 
-long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op)
+#endif /* !COMPAT */
+
+ret_t do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op)
 {
     xen_flask_op_t op;
     int rv;
@@ -763,3 +784,52 @@ long do_flask_op(XEN_GUEST_HANDLE_PARAM(
  out:
     return rv;
 }
+
+#ifndef COMPAT
+#undef _copy_to_guest
+#define _copy_to_guest copy_to_compat
+#undef _copy_from_guest
+#define _copy_from_guest copy_from_compat
+
+#include <compat/event_channel.h>
+#include <compat/xsm/flask_op.h>
+
+CHECK_flask_access;
+CHECK_flask_cache_stats;
+CHECK_flask_hash_stats;
+CHECK_flask_ocontext;
+CHECK_flask_peersid;
+CHECK_flask_relabel;
+CHECK_flask_setavc_threshold;
+CHECK_flask_setenforce;
+CHECK_flask_transition;
+
+#define COMPAT
+#define flask_copyin_string(ch, pb, sz, mx) ({ \
+	XEN_GUEST_HANDLE_PARAM(char) gh; \
+	guest_from_compat_handle(gh, ch); \
+	flask_copyin_string(gh, pb, sz, mx); \
+})
+
+#define xen_flask_load compat_flask_load
+#define flask_security_load compat_security_load
+
+#define xen_flask_userlist compat_flask_userlist
+#define flask_security_user compat_security_user
+
+#define xen_flask_sid_context compat_flask_sid_context
+#define flask_security_context compat_security_context
+#define flask_security_sid compat_security_sid
+
+#define xen_flask_boolean compat_flask_boolean
+#define flask_security_resolve_bool compat_security_resolve_bool
+#define flask_security_get_bool compat_security_get_bool
+#define flask_security_set_bool compat_security_set_bool
+
+#define xen_flask_op_t compat_flask_op_t
+#undef ret_t
+#define ret_t int
+#define do_flask_op compat_flask_op
+
+#include "flask_op.c"
+#endif
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1461,6 +1461,7 @@ static int flask_map_gmfn_foreign(struct
 #endif
 
 long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
+int compat_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op);
 
 static struct xsm_operations flask_ops = {
     .security_domaininfo = flask_security_domaininfo,
@@ -1535,6 +1536,9 @@ static struct xsm_operations flask_ops =
     .hvm_param_nested = flask_hvm_param_nested,
 
     .do_xsm_op = do_flask_op,
+#ifdef CONFIG_COMPAT
+    .do_compat_op = compat_flask_op,
+#endif
 
     .add_to_physmap = flask_add_to_physmap,
     .remove_from_physmap = flask_remove_from_physmap,
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -116,4 +116,9 @@ long do_xsm_op (XEN_GUEST_HANDLE_PARAM(x
     return xsm_do_xsm_op(op);
 }
 
-
+#ifdef CONFIG_COMPAT
+int compat_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
+{
+    return xsm_do_compat_op(op);
+}
+#endif

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/4] flask: XSA-84 follow-ups
  2014-02-07  9:41 [PATCH 0/4] flask: XSA-84 follow-ups Jan Beulich
                   ` (3 preceding siblings ...)
  2014-02-07  9:48 ` [PATCH 4/4] flask: add compat mode guest support Jan Beulich
@ 2014-02-07 11:29 ` George Dunlap
  2014-02-10 20:22 ` Daniel De Graaf
  5 siblings, 0 replies; 13+ messages in thread
From: George Dunlap @ 2014-02-07 11:29 UTC (permalink / raw)
  To: Jan Beulich, xen-devel; +Cc: dgdegra

On 02/07/2014 09:41 AM, Jan Beulich wrote:
> 1: fix memory leaks
> 2: fix error propagation from flask_security_set_bool()
> 3: check permissions first thing in flask_security_set_bool()
> 4: add compat mode guest support
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Release-wise, I would think that 1-3 should certainly go in. While I'd
> like 4 to be in for 4.4 too, I realize that's a little more intrusive than
> one would want at this point.

I agree on 1-3:

Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com>

Re #4: This is something we can pull in for 4.4.1, right?

If nobody has noticed it missing, it's not exactly a burning feature.  
If it's possible to wait for 4.4.1 I'd rather do so.

  -George

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/4] flask: fix memory leaks
  2014-02-07  9:46 ` [PATCH 1/4] flask: fix memory leaks Jan Beulich
@ 2014-02-07 12:51   ` Andrew Cooper
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2014-02-07 12:51 UTC (permalink / raw)
  To: Jan Beulich; +Cc: George Dunlap, xen-devel, dgdegra


[-- Attachment #1.1: Type: text/plain, Size: 1264 bytes --]

On 07/02/14 09:46, Jan Beulich wrote:
> Plus, in the case of security_preserve_bools(), prevent double freeing
> in the case of security_get_bools() failing.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -347,6 +347,7 @@ static int flask_security_set_bool(struc
>  
>          if ( arg->bool_id >= num )
>          {
> +            xfree(values);
>              rv = -ENOENT;
>              goto out;
>          }
> --- a/xen/xsm/flask/ss/services.c
> +++ b/xen/xsm/flask/ss/services.c
> @@ -1902,6 +1902,7 @@ err:
>      {
>          for ( i = 0; i < *len; i++ )
>              xfree((*names)[i]);
> +        xfree(*names);
>      }
>      xfree(*values);
>      goto out;
> @@ -2011,7 +2012,7 @@ static int security_preserve_bools(struc
>  
>      rc = security_get_bools(&nbools, &bnames, &bvalues, NULL);
>      if ( rc )
> -        goto out;
> +        return rc;
>      for ( i = 0; i < nbools; i++ )
>      {
>          booldatum = hashtab_search(p->p_bools.table, bnames[i]);
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 2147 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] flask: fix error propagation from flask_security_set_bool()
  2014-02-07  9:47 ` [PATCH 2/4] flask: fix error propagation from flask_security_set_bool() Jan Beulich
@ 2014-02-07 12:56   ` Andrew Cooper
  2014-02-07 13:04     ` Jan Beulich
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Cooper @ 2014-02-07 12:56 UTC (permalink / raw)
  To: Jan Beulich; +Cc: George Dunlap, xen-devel, dgdegra


[-- Attachment #1.1: Type: text/plain, Size: 1071 bytes --]

On 07/02/14 09:47, Jan Beulich wrote:
> The function should return an error when flask_security_make_bools() as

when flask_security_make_bools() fails ?

> well as when the input ID is out of range.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -364,9 +364,10 @@ static int flask_security_set_bool(struc
>      else
>      {
>          if ( !bool_pending_values )
> -            flask_security_make_bools();
> -
> -        if ( arg->bool_id >= bool_num )
> +            rv = flask_security_make_bools();
> +        if ( !rv && arg->bool_id >= bool_num )

Surely you want "rv || arg->" if you want to catch both
flask_security_make_bools() failing as well as the input ID being out of
range?

~Andrew

> +            rv = -ENOENT;
> +        if ( rv )
>              goto out;
>  
>          bool_pending_values[arg->bool_id] = !!(arg->new_value);
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 2186 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/4] flask: check permissions first thing in flask_security_set_bool()
  2014-02-07  9:47 ` [PATCH 3/4] flask: check permissions first thing in flask_security_set_bool() Jan Beulich
@ 2014-02-07 12:57   ` Andrew Cooper
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2014-02-07 12:57 UTC (permalink / raw)
  To: Jan Beulich; +Cc: George Dunlap, xen-devel, dgdegra


[-- Attachment #1.1: Type: text/plain, Size: 849 bytes --]

On 07/02/14 09:47, Jan Beulich wrote:
> Nothing else should be done if the caller isn't permitted to set
> boolean values.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -326,11 +326,11 @@ static int flask_security_set_bool(struc
>  {
>      int rv;
>  
> -    rv = flask_security_resolve_bool(arg);
> +    rv = domain_has_security(current->domain, SECURITY__SETBOOL);
>      if ( rv )
>          return rv;
>  
> -    rv = domain_has_security(current->domain, SECURITY__SETBOOL);
> +    rv = flask_security_resolve_bool(arg);
>      if ( rv )
>          return rv;
>  
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 1737 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] flask: fix error propagation from flask_security_set_bool()
  2014-02-07 12:56   ` Andrew Cooper
@ 2014-02-07 13:04     ` Jan Beulich
  2014-02-07 13:07       ` Andrew Cooper
  0 siblings, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2014-02-07 13:04 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: George Dunlap, xen-devel, dgdegra

>>> On 07.02.14 at 13:56, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 07/02/14 09:47, Jan Beulich wrote:
>> The function should return an error when flask_security_make_bools() as
> 
> when flask_security_make_bools() fails ?

Oops, yes of course. Corrected.

>> --- a/xen/xsm/flask/flask_op.c
>> +++ b/xen/xsm/flask/flask_op.c
>> @@ -364,9 +364,10 @@ static int flask_security_set_bool(struc
>>      else
>>      {
>>          if ( !bool_pending_values )
>> -            flask_security_make_bools();
>> -
>> -        if ( arg->bool_id >= bool_num )
>> +            rv = flask_security_make_bools();
>> +        if ( !rv && arg->bool_id >= bool_num )
> 
> Surely you want "rv || arg->" if you want to catch both
> flask_security_make_bools() failing as well as the input ID being out of
> range?

Yes, which is what the code does - it just cares to not clobber "rv"
if that got already set non-zero from the function call. See the
context below.

Jan

>> +            rv = -ENOENT;
>> +        if ( rv )
>>              goto out;
>>  
>>          bool_pending_values[arg->bool_id] = !!(arg->new_value);
>>
>>
>>
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org 
>> http://lists.xen.org/xen-devel 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/4] flask: fix error propagation from flask_security_set_bool()
  2014-02-07 13:04     ` Jan Beulich
@ 2014-02-07 13:07       ` Andrew Cooper
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2014-02-07 13:07 UTC (permalink / raw)
  To: Jan Beulich; +Cc: George Dunlap, xen-devel, dgdegra

On 07/02/14 13:04, Jan Beulich wrote:
>>>> On 07.02.14 at 13:56, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 07/02/14 09:47, Jan Beulich wrote:
>>> The function should return an error when flask_security_make_bools() as
>> when flask_security_make_bools() fails ?
> Oops, yes of course. Corrected.
>
>>> --- a/xen/xsm/flask/flask_op.c
>>> +++ b/xen/xsm/flask/flask_op.c
>>> @@ -364,9 +364,10 @@ static int flask_security_set_bool(struc
>>>      else
>>>      {
>>>          if ( !bool_pending_values )
>>> -            flask_security_make_bools();
>>> -
>>> -        if ( arg->bool_id >= bool_num )
>>> +            rv = flask_security_make_bools();
>>> +        if ( !rv && arg->bool_id >= bool_num )
>> Surely you want "rv || arg->" if you want to catch both
>> flask_security_make_bools() failing as well as the input ID being out of
>> range?
> Yes, which is what the code does - it just cares to not clobber "rv"
> if that got already set non-zero from the function call. See the
> context below.
>
> Jan

Ah yes.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
>>> +            rv = -ENOENT;
>>> +        if ( rv )
>>>              goto out;
>>>  
>>>          bool_pending_values[arg->bool_id] = !!(arg->new_value);
>>>
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xen.org 
>>> http://lists.xen.org/xen-devel 
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/4] flask: XSA-84 follow-ups
  2014-02-07  9:41 [PATCH 0/4] flask: XSA-84 follow-ups Jan Beulich
                   ` (4 preceding siblings ...)
  2014-02-07 11:29 ` [PATCH 0/4] flask: XSA-84 follow-ups George Dunlap
@ 2014-02-10 20:22 ` Daniel De Graaf
  2014-02-11  8:02   ` Jan Beulich
  5 siblings, 1 reply; 13+ messages in thread
From: Daniel De Graaf @ 2014-02-10 20:22 UTC (permalink / raw)
  To: Jan Beulich, xen-devel; +Cc: George Dunlap

On 02/07/2014 04:41 AM, Jan Beulich wrote:
> 1: fix memory leaks
> 2: fix error propagation from flask_security_set_bool()
> 3: check permissions first thing in flask_security_set_bool()
> 4: add compat mode guest support
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Release-wise, I would think that 1-3 should certainly go in. While I'd
> like 4 to be in for 4.4 too, I realize that's a little more intrusive than
> one would want at this point.
>
> Jan

All four patches look correct to me. I assume the movement of the flask_security_commit_bools inside the #ifdef is made possible by
the xlat.lst parsing, but didn't look too closely at how that was
done.

Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>

Re: what goes in release - I agree that #4 would be nice but I wouldn't
push too hard to make an exception for it. The users of the XSM interface
would primarily be toolstack and related domains where a requirement to
be 64-bit should not be too restrictive (not to say this shouldn't be
fixed, of course).

-- 
Daniel De Graaf
National Security Agency

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/4] flask: XSA-84 follow-ups
  2014-02-10 20:22 ` Daniel De Graaf
@ 2014-02-11  8:02   ` Jan Beulich
  0 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2014-02-11  8:02 UTC (permalink / raw)
  To: Daniel De Graaf; +Cc: George Dunlap, xen-devel

>>> On 10.02.14 at 21:22, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
> On 02/07/2014 04:41 AM, Jan Beulich wrote:
>> 1: fix memory leaks
>> 2: fix error propagation from flask_security_set_bool()
>> 3: check permissions first thing in flask_security_set_bool()
>> 4: add compat mode guest support
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> Release-wise, I would think that 1-3 should certainly go in. While I'd
>> like 4 to be in for 4.4 too, I realize that's a little more intrusive than
>> one would want at this point.
> 
> All four patches look correct to me. I assume the movement of the 
> flask_security_commit_bools inside the #ifdef is made possible by
> the xlat.lst parsing, but didn't look too closely at how that was
> done.

No, that has nothing to do with the xlat.lst parsing. It's solely with
the goal of having one less #ifndef COMPAT code section (as we
need static helper functions to be defined exactly once in the whole
compilation unit, yet the file includes itself after #define-ing COMPAT,
all these functions must be inside such conditionals).

> Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> Re: what goes in release - I agree that #4 would be nice but I wouldn't
> push too hard to make an exception for it. The users of the XSM interface
> would primarily be toolstack and related domains where a requirement to
> be 64-bit should not be too restrictive (not to say this shouldn't be
> fixed, of course).

With George's feedback on the same matter, I already pushed the
patch back to my 4.5 queue.

Jan

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-02-11  8:02 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-07  9:41 [PATCH 0/4] flask: XSA-84 follow-ups Jan Beulich
2014-02-07  9:46 ` [PATCH 1/4] flask: fix memory leaks Jan Beulich
2014-02-07 12:51   ` Andrew Cooper
2014-02-07  9:47 ` [PATCH 2/4] flask: fix error propagation from flask_security_set_bool() Jan Beulich
2014-02-07 12:56   ` Andrew Cooper
2014-02-07 13:04     ` Jan Beulich
2014-02-07 13:07       ` Andrew Cooper
2014-02-07  9:47 ` [PATCH 3/4] flask: check permissions first thing in flask_security_set_bool() Jan Beulich
2014-02-07 12:57   ` Andrew Cooper
2014-02-07  9:48 ` [PATCH 4/4] flask: add compat mode guest support Jan Beulich
2014-02-07 11:29 ` [PATCH 0/4] flask: XSA-84 follow-ups George Dunlap
2014-02-10 20:22 ` Daniel De Graaf
2014-02-11  8:02   ` Jan Beulich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).