Mailing List Archive

[PATCH 3/6] xsm: enabling xsm to always be included
The only difference between !CONFIG_XSM and CONFIG_XSM with !CONFIG_XSM_SILO and !CONFIG_XSM_FLASK
is whether the XSM hooks in dummy.h are called as static inline functions or as function
pointers to static functions. As such this commit,
* eliminates CONFIG_XSM
* introduces CONFIG_XSM_EVTCHN_LABELING as replacement for enabling event channel labels
* makes CONFIG_XSM_SILO AND CONFIG_XSM_FLASK default to no

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
xen/common/Kconfig | 55 ++++-----
xen/include/xen/sched.h | 2 +-
xen/include/xsm/xsm-core.h | 26 ----
xen/include/xsm/xsm.h | 8 --
xen/xsm/Makefile | 4 +-
xen/xsm/dummy.c | 4 +-
xen/{include => }/xsm/dummy.h | 220 ++++++++++++++++------------------
xen/xsm/silo.c | 17 +--
xen/xsm/xsm_core.c | 4 -
9 files changed, 142 insertions(+), 198 deletions(-)
rename xen/{include => }/xsm/dummy.h (63%)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 0ddd18e11a..203ad7ea23 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -197,22 +197,33 @@ config XENOPROF

If unsure, say Y.

-config XSM
- bool "Xen Security Modules support"
- default ARM
- ---help---
- Enables the security framework known as Xen Security Modules which
- allows administrators fine-grained control over a Xen domain and
- its capabilities by defining permissible interactions between domains,
- the hypervisor itself, and related resources such as memory and
- devices.
+menu "Xen Security Modules"

- If unsure, say N.
+choice
+ prompt "Default XSM module"
+ default XSM_SILO_DEFAULT if XSM_SILO && ARM
+ default XSM_FLASK_DEFAULT if XSM_FLASK
+ default XSM_SILO_DEFAULT if XSM_SILO
+ default XSM_DUMMY_DEFAULT
+ config XSM_DUMMY_DEFAULT
+ bool "Match non-XSM behavior"
+ config XSM_FLASK_DEFAULT
+ bool "FLux Advanced Security Kernel" if XSM_FLASK
+ config XSM_SILO_DEFAULT
+ bool "SILO" if XSM_SILO
+endchoice
+
+config XSM_EVTCHN_LABELING
+ bool "Enables security labeling of event channels"
+ default n
+ ---help---
+ This enables an XSM module to label and enforce access control over
+ event channels.

config XSM_FLASK
- def_bool y
+ def_bool n
prompt "FLux Advanced Security Kernel support"
- depends on XSM
+ select XSM_EVTCHN_LABELING
---help---
Enables FLASK (FLux Advanced Security Kernel) as the access control
mechanism used by the XSM framework. This provides a mandatory access
@@ -250,9 +261,8 @@ config XSM_FLASK_POLICY
If unsure, say Y.

config XSM_SILO
- def_bool y
+ def_bool n
prompt "SILO support"
- depends on XSM
---help---
Enables SILO as the access control mechanism used by the XSM framework.
This is not the default module, add boot parameter xsm=silo to choose
@@ -261,25 +271,12 @@ config XSM_SILO

If unsure, say Y.

-choice
- prompt "Default XSM implementation"
- depends on XSM
- default XSM_SILO_DEFAULT if XSM_SILO && ARM
- default XSM_FLASK_DEFAULT if XSM_FLASK
- default XSM_SILO_DEFAULT if XSM_SILO
- default XSM_DUMMY_DEFAULT
- config XSM_DUMMY_DEFAULT
- bool "Match non-XSM behavior"
- config XSM_FLASK_DEFAULT
- bool "FLux Advanced Security Kernel" if XSM_FLASK
- config XSM_SILO_DEFAULT
- bool "SILO" if XSM_SILO
-endchoice
+endmenu

config LATE_HWDOM
bool "Dedicated hardware domain"
default n
- depends on XSM && X86
+ depends on XSM_FLASK && X86
---help---
Allows the creation of a dedicated hardware domain distinct from
domain 0 that manages devices without needing access to other
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 3982167144..e92d2cdeab 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -120,7 +120,7 @@ struct evtchn
unsigned short notify_vcpu_id; /* VCPU for local delivery notification */
uint32_t fifo_lastq; /* Data for identifying last queue. */

-#ifdef CONFIG_XSM
+#ifdef CONFIG_XSM_EVTCHN_LABELING
union {
#ifdef XSM_NEED_GENERIC_EVTCHN_SSID
/*
diff --git a/xen/include/xsm/xsm-core.h b/xen/include/xsm/xsm-core.h
index 5297c73fe6..e3718bc62d 100644
--- a/xen/include/xsm/xsm-core.h
+++ b/xen/include/xsm/xsm-core.h
@@ -189,8 +189,6 @@ struct xsm_operations {
#endif
};

-#ifdef CONFIG_XSM
-
#ifdef CONFIG_MULTIBOOT
extern int xsm_multiboot_init(unsigned long *module_map,
const multiboot_info_t *mbi);
@@ -235,28 +233,4 @@ extern void silo_init(void);
static inline void silo_init(void) {}
#endif

-#else /* CONFIG_XSM */
-
-#ifdef CONFIG_MULTIBOOT
-static inline int xsm_multiboot_init (unsigned long *module_map,
- const multiboot_info_t *mbi)
-{
- return 0;
-}
-#endif
-
-#ifdef CONFIG_HAS_DEVICE_TREE
-static inline int xsm_dt_init(void)
-{
- return 0;
-}
-
-static inline bool has_xsm_magic(paddr_t start)
-{
- return false;
-}
-#endif /* CONFIG_HAS_DEVICE_TREE */
-
-#endif /* CONFIG_XSM */
-
#endif /* __XSM_CORE_H */
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index ecbdee2c7d..69f68300e2 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -22,8 +22,6 @@
#include <xen/multiboot.h>
#include <xsm/xsm-core.h>

-#ifdef CONFIG_XSM
-
extern struct xsm_operations xsm_ops;

static inline void xsm_security_domaininfo (struct domain *d,
@@ -559,10 +557,4 @@ static inline int xsm_argo_send(const struct domain *d, const struct domain *t)

#endif /* CONFIG_ARGO */

-#else /* CONFIG_XSM */
-
-#include <xsm/dummy.h>
-
-#endif /* CONFIG_XSM */
-
#endif /* __XSM_H */
diff --git a/xen/xsm/Makefile b/xen/xsm/Makefile
index cf0a728f1c..0059794dd5 100644
--- a/xen/xsm/Makefile
+++ b/xen/xsm/Makefile
@@ -1,6 +1,6 @@
obj-y += xsm_core.o
-obj-$(CONFIG_XSM) += xsm_policy.o
-obj-$(CONFIG_XSM) += dummy.o
+obj-y += xsm_policy.o
+obj-y += dummy.o
obj-$(CONFIG_XSM_SILO) += silo.o

obj-$(CONFIG_XSM_FLASK) += flask/
diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c
index de4d6cf2cf..bfd8b96f08 100644
--- a/xen/xsm/dummy.c
+++ b/xen/xsm/dummy.c
@@ -10,12 +10,12 @@
* as published by the Free Software Foundation.
*/

-#include <xsm/dummy.h>
+#include "dummy.h"

#define set_to_dummy_if_null(ops, function) \
do { \
if ( !ops->function ) \
- ops->function = xsm_##function; \
+ ops->function = dummy_##function; \
} while (0)

void __init xsm_fixup_ops (struct xsm_operations *ops)
diff --git a/xen/include/xsm/dummy.h b/xen/xsm/dummy.h
similarity index 63%
rename from xen/include/xsm/dummy.h
rename to xen/xsm/dummy.h
index c445c5681b..7e2bb09dac 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/xsm/dummy.h
@@ -42,12 +42,10 @@ static inline void __xsm_action_mismatch_detected(void)
void __xsm_action_mismatch_detected(void);
#endif

-#ifdef CONFIG_XSM
-
-/* In CONFIG_XSM builds, this header file is included from xsm/dummy.c, and
- * contains static (not inline) functions compiled to the dummy XSM module.
- * There is no xsm_default_t argument available, so the value from the assertion
- * is used to initialize the variable.
+/* This header file is included from xsm/dummy.c, and contains static (not
+ * inline) functions compiled to the dummy XSM module. There is no
+ * xsm_default_t argument available, so the value from the assertion is used to
+ * initialize the variable.
*/
#define XSM_INLINE __maybe_unused

@@ -55,20 +53,6 @@ void __xsm_action_mismatch_detected(void);
#define XSM_DEFAULT_VOID void
#define XSM_ASSERT_ACTION(def) xsm_default_t action = def; (void)action

-#else /* CONFIG_XSM */
-
-/* In !CONFIG_XSM builds, this header file is included from xsm/xsm.h, and
- * contains inline functions for each XSM hook. These functions also perform
- * compile-time checks on the xsm_default_t argument to ensure that the behavior
- * of the dummy XSM module is the same as the behavior with XSM disabled.
- */
-#define XSM_INLINE always_inline
-#define XSM_DEFAULT_ARG xsm_default_t action,
-#define XSM_DEFAULT_VOID xsm_default_t action
-#define XSM_ASSERT_ACTION(def) LINKER_BUG_ON(def != action)
-
-#endif /* CONFIG_XSM */
-
static always_inline int xsm_default_action(
xsm_default_t action, struct domain *src, struct domain *target)
{
@@ -98,43 +82,43 @@ static always_inline int xsm_default_action(
}
}

-static XSM_INLINE void xsm_security_domaininfo(struct domain *d,
+static XSM_INLINE void dummy_security_domaininfo(struct domain *d,
struct xen_domctl_getdomaininfo *info)
{
return;
}

-static XSM_INLINE int xsm_domain_create(XSM_DEFAULT_ARG struct domain *d, u32 ssidref)
+static XSM_INLINE int dummy_domain_create(XSM_DEFAULT_ARG struct domain *d, u32 ssidref)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_getdomaininfo(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_getdomaininfo(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_domctl_scheduler_op(XSM_DEFAULT_ARG struct domain *d, int cmd)
+static XSM_INLINE int dummy_domctl_scheduler_op(XSM_DEFAULT_ARG struct domain *d, int cmd)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_sysctl_scheduler_op(XSM_DEFAULT_ARG int cmd)
+static XSM_INLINE int dummy_sysctl_scheduler_op(XSM_DEFAULT_ARG int cmd)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_set_target(XSM_DEFAULT_ARG struct domain *d, struct domain *e)
+static XSM_INLINE int dummy_set_target(XSM_DEFAULT_ARG struct domain *d, struct domain *e)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_domctl(XSM_DEFAULT_ARG struct domain *d, int cmd)
+static XSM_INLINE int dummy_domctl(XSM_DEFAULT_ARG struct domain *d, int cmd)
{
XSM_ASSERT_ACTION(XSM_OTHER);
switch ( cmd )
@@ -151,85 +135,85 @@ static XSM_INLINE int xsm_domctl(XSM_DEFAULT_ARG struct domain *d, int cmd)
}
}

-static XSM_INLINE int xsm_sysctl(XSM_DEFAULT_ARG int cmd)
+static XSM_INLINE int dummy_sysctl(XSM_DEFAULT_ARG int cmd)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_readconsole(XSM_DEFAULT_ARG uint32_t clear)
+static XSM_INLINE int dummy_readconsole(XSM_DEFAULT_ARG uint32_t clear)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_alloc_security_domain(struct domain *d)
+static XSM_INLINE int dummy_alloc_security_domain(struct domain *d)
{
return 0;
}

-static XSM_INLINE void xsm_free_security_domain(struct domain *d)
+static XSM_INLINE void dummy_free_security_domain(struct domain *d)
{
return;
}

-static XSM_INLINE int xsm_grant_mapref(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2,
+static XSM_INLINE int dummy_grant_mapref(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2,
uint32_t flags)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_grant_unmapref(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
+static XSM_INLINE int dummy_grant_unmapref(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_grant_setup(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
+static XSM_INLINE int dummy_grant_setup(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_grant_transfer(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
+static XSM_INLINE int dummy_grant_transfer(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_grant_copy(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
+static XSM_INLINE int dummy_grant_copy(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_grant_query_size(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
+static XSM_INLINE int dummy_grant_query_size(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_memory_exchange(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_memory_exchange(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_memory_adjust_reservation(XSM_DEFAULT_ARG struct domain *d1,
+static XSM_INLINE int dummy_memory_adjust_reservation(XSM_DEFAULT_ARG struct domain *d1,
struct domain *d2)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_memory_stat_reservation(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
+static XSM_INLINE int dummy_memory_stat_reservation(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_console_io(XSM_DEFAULT_ARG struct domain *d, int cmd)
+static XSM_INLINE int dummy_console_io(XSM_DEFAULT_ARG struct domain *d, int cmd)
{
XSM_ASSERT_ACTION(XSM_OTHER);
if ( d->is_console )
@@ -241,129 +225,129 @@ static XSM_INLINE int xsm_console_io(XSM_DEFAULT_ARG struct domain *d, int cmd)
return xsm_default_action(XSM_PRIV, d, NULL);
}

-static XSM_INLINE int xsm_profile(XSM_DEFAULT_ARG struct domain *d, int op)
+static XSM_INLINE int dummy_profile(XSM_DEFAULT_ARG struct domain *d, int op)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, d, NULL);
}

-static XSM_INLINE int xsm_kexec(XSM_DEFAULT_VOID)
+static XSM_INLINE int dummy_kexec(XSM_DEFAULT_VOID)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_schedop_shutdown(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
+static XSM_INLINE int dummy_schedop_shutdown(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
{
XSM_ASSERT_ACTION(XSM_DM_PRIV);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_memory_pin_page(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2,
+static XSM_INLINE int dummy_memory_pin_page(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2,
struct page_info *page)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_claim_pages(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_claim_pages(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_evtchn_unbound(XSM_DEFAULT_ARG struct domain *d, struct evtchn *chn,
+static XSM_INLINE int dummy_evtchn_unbound(XSM_DEFAULT_ARG struct domain *d, struct evtchn *chn,
domid_t id2)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_evtchn_interdomain(XSM_DEFAULT_ARG struct domain *d1, struct evtchn
+static XSM_INLINE int dummy_evtchn_interdomain(XSM_DEFAULT_ARG struct domain *d1, struct evtchn
*chan1, struct domain *d2, struct evtchn *chan2)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE void xsm_evtchn_close_post(struct evtchn *chn)
+static XSM_INLINE void dummy_evtchn_close_post(struct evtchn *chn)
{
return;
}

-static XSM_INLINE int xsm_evtchn_send(XSM_DEFAULT_ARG struct domain *d, struct evtchn *chn)
+static XSM_INLINE int dummy_evtchn_send(XSM_DEFAULT_ARG struct domain *d, struct evtchn *chn)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, d, NULL);
}

-static XSM_INLINE int xsm_evtchn_status(XSM_DEFAULT_ARG struct domain *d, struct evtchn *chn)
+static XSM_INLINE int dummy_evtchn_status(XSM_DEFAULT_ARG struct domain *d, struct evtchn *chn)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_evtchn_reset(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
+static XSM_INLINE int dummy_evtchn_reset(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_alloc_security_evtchns(
+static XSM_INLINE int dummy_alloc_security_evtchns(
struct evtchn chn[], unsigned int nr)
{
return 0;
}

-static XSM_INLINE void xsm_free_security_evtchns(
+static XSM_INLINE void dummy_free_security_evtchns(
struct evtchn chn[], unsigned int nr)
{
return;
}

-static XSM_INLINE char *xsm_show_security_evtchn(struct domain *d, const struct evtchn *chn)
+static XSM_INLINE char *dummy_show_security_evtchn(struct domain *d, const struct evtchn *chn)
{
return NULL;
}

-static XSM_INLINE int xsm_init_hardware_domain(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_init_hardware_domain(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_get_pod_target(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_get_pod_target(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_set_pod_target(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_set_pod_target(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_get_vnumainfo(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_get_vnumainfo(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, current->domain, d);
}

#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_PCI)
-static XSM_INLINE int xsm_get_device_group(XSM_DEFAULT_ARG uint32_t machine_bdf)
+static XSM_INLINE int dummy_get_device_group(XSM_DEFAULT_ARG uint32_t machine_bdf)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_assign_device(XSM_DEFAULT_ARG struct domain *d, uint32_t machine_bdf)
+static XSM_INLINE int dummy_assign_device(XSM_DEFAULT_ARG struct domain *d, uint32_t machine_bdf)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_deassign_device(XSM_DEFAULT_ARG struct domain *d, uint32_t machine_bdf)
+static XSM_INLINE int dummy_deassign_device(XSM_DEFAULT_ARG struct domain *d, uint32_t machine_bdf)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
@@ -372,14 +356,14 @@ static XSM_INLINE int xsm_deassign_device(XSM_DEFAULT_ARG struct domain *d, uint
#endif /* HAS_PASSTHROUGH && HAS_PCI */

#if defined(CONFIG_HAS_PASSTHROUGH) && defined(CONFIG_HAS_DEVICE_TREE)
-static XSM_INLINE int xsm_assign_dtdevice(XSM_DEFAULT_ARG struct domain *d,
+static XSM_INLINE int dummy_assign_dtdevice(XSM_DEFAULT_ARG struct domain *d,
const char *dtpath)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_deassign_dtdevice(XSM_DEFAULT_ARG struct domain *d,
+static XSM_INLINE int dummy_deassign_dtdevice(XSM_DEFAULT_ARG struct domain *d,
const char *dtpath)
{
XSM_ASSERT_ACTION(XSM_HOOK);
@@ -388,134 +372,134 @@ static XSM_INLINE int xsm_deassign_dtdevice(XSM_DEFAULT_ARG struct domain *d,

#endif /* HAS_PASSTHROUGH && HAS_DEVICE_TREE */

-static XSM_INLINE int xsm_resource_plug_core(XSM_DEFAULT_VOID)
+static XSM_INLINE int dummy_resource_plug_core(XSM_DEFAULT_VOID)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_resource_unplug_core(XSM_DEFAULT_VOID)
+static XSM_INLINE int dummy_resource_unplug_core(XSM_DEFAULT_VOID)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_resource_plug_pci(XSM_DEFAULT_ARG uint32_t machine_bdf)
+static XSM_INLINE int dummy_resource_plug_pci(XSM_DEFAULT_ARG uint32_t machine_bdf)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_resource_unplug_pci(XSM_DEFAULT_ARG uint32_t machine_bdf)
+static XSM_INLINE int dummy_resource_unplug_pci(XSM_DEFAULT_ARG uint32_t machine_bdf)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_resource_setup_pci(XSM_DEFAULT_ARG uint32_t machine_bdf)
+static XSM_INLINE int dummy_resource_setup_pci(XSM_DEFAULT_ARG uint32_t machine_bdf)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_resource_setup_gsi(XSM_DEFAULT_ARG int gsi)
+static XSM_INLINE int dummy_resource_setup_gsi(XSM_DEFAULT_ARG int gsi)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_resource_setup_misc(XSM_DEFAULT_VOID)
+static XSM_INLINE int dummy_resource_setup_misc(XSM_DEFAULT_VOID)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_page_offline(XSM_DEFAULT_ARG uint32_t cmd)
+static XSM_INLINE int dummy_page_offline(XSM_DEFAULT_ARG uint32_t cmd)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_hypfs_op(XSM_DEFAULT_VOID)
+static XSM_INLINE int dummy_hypfs_op(XSM_DEFAULT_VOID)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE long xsm_do_xsm_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
+static XSM_INLINE long dummy_do_xsm_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
{
return -ENOSYS;
}

#ifdef CONFIG_COMPAT
-static XSM_INLINE int xsm_do_compat_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
+static XSM_INLINE int dummy_do_compat_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
{
return -ENOSYS;
}
#endif

-static XSM_INLINE char *xsm_show_irq_sid(int irq)
+static XSM_INLINE char *dummy_show_irq_sid(int irq)
{
return NULL;
}

-static XSM_INLINE int xsm_map_domain_pirq(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_map_domain_pirq(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_DM_PRIV);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_map_domain_irq(XSM_DEFAULT_ARG struct domain *d,
+static XSM_INLINE int dummy_map_domain_irq(XSM_DEFAULT_ARG struct domain *d,
int irq, const void *data)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_unmap_domain_pirq(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_unmap_domain_pirq(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_DM_PRIV);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_bind_pt_irq(XSM_DEFAULT_ARG struct domain *d, struct xen_domctl_bind_pt_irq *bind)
+static XSM_INLINE int dummy_bind_pt_irq(XSM_DEFAULT_ARG struct domain *d, struct xen_domctl_bind_pt_irq *bind)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_unbind_pt_irq(XSM_DEFAULT_ARG struct domain *d, struct xen_domctl_bind_pt_irq *bind)
+static XSM_INLINE int dummy_unbind_pt_irq(XSM_DEFAULT_ARG struct domain *d, struct xen_domctl_bind_pt_irq *bind)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_unmap_domain_irq(XSM_DEFAULT_ARG struct domain *d,
+static XSM_INLINE int dummy_unmap_domain_irq(XSM_DEFAULT_ARG struct domain *d,
int irq, const void *data)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_irq_permission(XSM_DEFAULT_ARG struct domain *d, int pirq, uint8_t allow)
+static XSM_INLINE int dummy_irq_permission(XSM_DEFAULT_ARG struct domain *d, int pirq, uint8_t allow)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_iomem_permission(XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, uint8_t allow)
+static XSM_INLINE int dummy_iomem_permission(XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, uint8_t allow)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_iomem_mapping(XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, uint8_t allow)
+static XSM_INLINE int dummy_iomem_mapping(XSM_DEFAULT_ARG struct domain *d, uint64_t s, uint64_t e, uint8_t allow)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_pci_config_permission(XSM_DEFAULT_ARG struct domain *d, uint32_t machine_bdf,
+static XSM_INLINE int dummy_pci_config_permission(XSM_DEFAULT_ARG struct domain *d, uint32_t machine_bdf,
uint16_t start, uint16_t end,
uint8_t access)
{
@@ -523,43 +507,43 @@ static XSM_INLINE int xsm_pci_config_permission(XSM_DEFAULT_ARG struct domain *d
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_add_to_physmap(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
+static XSM_INLINE int dummy_add_to_physmap(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
+static XSM_INLINE int dummy_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1, struct domain *d2)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, d1, d2);
}

-static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, struct domain *t)
+static XSM_INLINE int dummy_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, struct domain *t)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, d, t);
}

-static XSM_INLINE int xsm_hvm_param(XSM_DEFAULT_ARG struct domain *d, unsigned long op)
+static XSM_INLINE int dummy_hvm_param(XSM_DEFAULT_ARG struct domain *d, unsigned long op)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_hvm_control(XSM_DEFAULT_ARG struct domain *d, unsigned long op)
+static XSM_INLINE int dummy_hvm_control(XSM_DEFAULT_ARG struct domain *d, unsigned long op)
{
XSM_ASSERT_ACTION(XSM_DM_PRIV);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_hvm_param_altp2mhvm(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_hvm_param_altp2mhvm(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_hvm_altp2mhvm_op(XSM_DEFAULT_ARG struct domain *d, uint64_t mode, uint32_t op)
+static XSM_INLINE int dummy_hvm_altp2mhvm_op(XSM_DEFAULT_ARG struct domain *d, uint64_t mode, uint32_t op)
{
XSM_ASSERT_ACTION(XSM_OTHER);

@@ -578,14 +562,14 @@ static XSM_INLINE int xsm_hvm_altp2mhvm_op(XSM_DEFAULT_ARG struct domain *d, uin
}
}

-static XSM_INLINE int xsm_vm_event_control(XSM_DEFAULT_ARG struct domain *d, int mode, int op)
+static XSM_INLINE int dummy_vm_event_control(XSM_DEFAULT_ARG struct domain *d, int mode, int op)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, d);
}

#ifdef CONFIG_MEM_ACCESS
-static XSM_INLINE int xsm_mem_access(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_mem_access(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_DM_PRIV);
return xsm_default_action(action, current->domain, d);
@@ -593,7 +577,7 @@ static XSM_INLINE int xsm_mem_access(XSM_DEFAULT_ARG struct domain *d)
#endif

#ifdef CONFIG_MEM_PAGING
-static XSM_INLINE int xsm_mem_paging(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_mem_paging(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_DM_PRIV);
return xsm_default_action(action, current->domain, d);
@@ -601,57 +585,57 @@ static XSM_INLINE int xsm_mem_paging(XSM_DEFAULT_ARG struct domain *d)
#endif

#ifdef CONFIG_MEM_SHARING
-static XSM_INLINE int xsm_mem_sharing(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_mem_sharing(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_DM_PRIV);
return xsm_default_action(action, current->domain, d);
}
#endif

-static XSM_INLINE int xsm_platform_op(XSM_DEFAULT_ARG uint32_t op)
+static XSM_INLINE int dummy_platform_op(XSM_DEFAULT_ARG uint32_t op)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, NULL);
}

#ifdef CONFIG_X86
-static XSM_INLINE int xsm_do_mca(XSM_DEFAULT_VOID)
+static XSM_INLINE int dummy_do_mca(XSM_DEFAULT_VOID)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_shadow_control(XSM_DEFAULT_ARG struct domain *d, uint32_t op)
+static XSM_INLINE int dummy_shadow_control(XSM_DEFAULT_ARG struct domain *d, uint32_t op)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_mem_sharing_op(XSM_DEFAULT_ARG struct domain *d, struct domain *cd, int op)
+static XSM_INLINE int dummy_mem_sharing_op(XSM_DEFAULT_ARG struct domain *d, struct domain *cd, int op)
{
XSM_ASSERT_ACTION(XSM_DM_PRIV);
return xsm_default_action(action, current->domain, cd);
}

-static XSM_INLINE int xsm_apic(XSM_DEFAULT_ARG struct domain *d, int cmd)
+static XSM_INLINE int dummy_apic(XSM_DEFAULT_ARG struct domain *d, int cmd)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, d, NULL);
}

-static XSM_INLINE int xsm_machine_memory_map(XSM_DEFAULT_VOID)
+static XSM_INLINE int dummy_machine_memory_map(XSM_DEFAULT_VOID)
{
XSM_ASSERT_ACTION(XSM_PRIV);
return xsm_default_action(action, current->domain, NULL);
}

-static XSM_INLINE int xsm_domain_memory_map(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_domain_memory_map(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_mmu_update(XSM_DEFAULT_ARG struct domain *d, struct domain *t,
+static XSM_INLINE int dummy_mmu_update(XSM_DEFAULT_ARG struct domain *d, struct domain *t,
struct domain *f, uint32_t flags)
{
int rc = 0;
@@ -663,38 +647,38 @@ static XSM_INLINE int xsm_mmu_update(XSM_DEFAULT_ARG struct domain *d, struct do
return rc;
}

-static XSM_INLINE int xsm_mmuext_op(XSM_DEFAULT_ARG struct domain *d, struct domain *f)
+static XSM_INLINE int dummy_mmuext_op(XSM_DEFAULT_ARG struct domain *d, struct domain *f)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, d, f);
}

-static XSM_INLINE int xsm_update_va_mapping(XSM_DEFAULT_ARG struct domain *d, struct domain *f,
+static XSM_INLINE int dummy_update_va_mapping(XSM_DEFAULT_ARG struct domain *d, struct domain *f,
l1_pgentry_t pte)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, d, f);
}

-static XSM_INLINE int xsm_priv_mapping(XSM_DEFAULT_ARG struct domain *d, struct domain *t)
+static XSM_INLINE int dummy_priv_mapping(XSM_DEFAULT_ARG struct domain *d, struct domain *t)
{
XSM_ASSERT_ACTION(XSM_TARGET);
return xsm_default_action(action, d, t);
}

-static XSM_INLINE int xsm_ioport_permission(XSM_DEFAULT_ARG struct domain *d, uint32_t s, uint32_t e, uint8_t allow)
+static XSM_INLINE int dummy_ioport_permission(XSM_DEFAULT_ARG struct domain *d, uint32_t s, uint32_t e, uint8_t allow)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_ioport_mapping(XSM_DEFAULT_ARG struct domain *d, uint32_t s, uint32_t e, uint8_t allow)
+static XSM_INLINE int dummy_ioport_mapping(XSM_DEFAULT_ARG struct domain *d, uint32_t s, uint32_t e, uint8_t allow)
{
XSM_ASSERT_ACTION(XSM_HOOK);
return xsm_default_action(action, current->domain, d);
}

-static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG struct domain *d, unsigned int op)
+static XSM_INLINE int dummy_pmu_op (XSM_DEFAULT_ARG struct domain *d, unsigned int op)
{
XSM_ASSERT_ACTION(XSM_OTHER);
switch ( op )
@@ -711,30 +695,30 @@ static XSM_INLINE int xsm_pmu_op (XSM_DEFAULT_ARG struct domain *d, unsigned int

#endif /* CONFIG_X86 */

-static XSM_INLINE int xsm_dm_op(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_dm_op(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_DM_PRIV);
return xsm_default_action(action, current->domain, d);
}

#ifdef CONFIG_ARGO
-static XSM_INLINE int xsm_argo_enable(const struct domain *d)
+static XSM_INLINE int dummy_argo_enable(const struct domain *d)
{
return 0;
}

-static XSM_INLINE int xsm_argo_register_single_source(const struct domain *d,
+static XSM_INLINE int dummy_argo_register_single_source(const struct domain *d,
const struct domain *t)
{
return 0;
}

-static XSM_INLINE int xsm_argo_register_any_source(const struct domain *d)
+static XSM_INLINE int dummy_argo_register_any_source(const struct domain *d)
{
return 0;
}

-static XSM_INLINE int xsm_argo_send(const struct domain *d,
+static XSM_INLINE int dummy_argo_send(const struct domain *d,
const struct domain *t)
{
return 0;
@@ -743,7 +727,7 @@ static XSM_INLINE int xsm_argo_send(const struct domain *d,
#endif /* CONFIG_ARGO */

#include <public/version.h>
-static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG uint32_t op)
+static XSM_INLINE int dummy_xen_version (XSM_DEFAULT_ARG uint32_t op)
{
XSM_ASSERT_ACTION(XSM_OTHER);
switch ( op )
@@ -767,7 +751,7 @@ static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG uint32_t op)
}
}

-static XSM_INLINE int xsm_domain_resource_map(XSM_DEFAULT_ARG struct domain *d)
+static XSM_INLINE int dummy_domain_resource_map(XSM_DEFAULT_ARG struct domain *d)
{
XSM_ASSERT_ACTION(XSM_DM_PRIV);
return xsm_default_action(action, current->domain, d);
diff --git a/xen/xsm/silo.c b/xen/xsm/silo.c
index b96dacd181..ebe5814efc 100644
--- a/xen/xsm/silo.c
+++ b/xen/xsm/silo.c
@@ -17,7 +17,8 @@
* You should have received a copy of the GNU General Public License along with
* this program; If not, see <http://www.gnu.org/licenses/>.
*/
-#include <xsm/dummy.h>
+#include <xsm/xsm-core.h>
+#include "dummy.h"

/*
* Check if inter-domain communication is allowed.
@@ -43,7 +44,7 @@ static int silo_evtchn_unbound(struct domain *d1, struct evtchn *chn,
else
{
if ( silo_mode_dom_check(d1, d2) )
- rc = xsm_evtchn_unbound(d1, chn, id2);
+ rc = dummy_evtchn_unbound(d1, chn, id2);
rcu_unlock_domain(d2);
}

@@ -54,7 +55,7 @@ static int silo_evtchn_interdomain(struct domain *d1, struct evtchn *chan1,
struct domain *d2, struct evtchn *chan2)
{
if ( silo_mode_dom_check(d1, d2) )
- return xsm_evtchn_interdomain(d1, chan1, d2, chan2);
+ return dummy_evtchn_interdomain(d1, chan1, d2, chan2);
return -EPERM;
}

@@ -62,21 +63,21 @@ static int silo_grant_mapref(struct domain *d1, struct domain *d2,
uint32_t flags)
{
if ( silo_mode_dom_check(d1, d2) )
- return xsm_grant_mapref(d1, d2, flags);
+ return dummy_grant_mapref(d1, d2, flags);
return -EPERM;
}

static int silo_grant_transfer(struct domain *d1, struct domain *d2)
{
if ( silo_mode_dom_check(d1, d2) )
- return xsm_grant_transfer(d1, d2);
+ return dummy_grant_transfer(d1, d2);
return -EPERM;
}

static int silo_grant_copy(struct domain *d1, struct domain *d2)
{
if ( silo_mode_dom_check(d1, d2) )
- return xsm_grant_copy(d1, d2);
+ return dummy_grant_copy(d1, d2);
return -EPERM;
}

@@ -86,14 +87,14 @@ static int silo_argo_register_single_source(const struct domain *d1,
const struct domain *d2)
{
if ( silo_mode_dom_check(d1, d2) )
- return xsm_argo_register_single_source(d1, d2);
+ return dummy_argo_register_single_source(d1, d2);
return -EPERM;
}

static int silo_argo_send(const struct domain *d1, const struct domain *d2)
{
if ( silo_mode_dom_check(d1, d2) )
- return xsm_argo_send(d1, d2);
+ return dummy_argo_send(d1, d2);
return -EPERM;
}

diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index acc1af7166..a51dc6f7dd 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -18,8 +18,6 @@
#include <xen/hypercall.h>
#include <xsm/xsm.h>

-#ifdef CONFIG_XSM
-
#ifdef CONFIG_MULTIBOOT
#include <asm/setup.h>
#endif
@@ -223,8 +221,6 @@ int __init register_xsm(struct xsm_operations *ops)
return 0;
}

-#endif
-
long do_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
{
return xsm_do_xsm_op(op);
--
2.20.1
Re: [PATCH 3/6] xsm: enabling xsm to always be included [ In reply to ]
On 18/06/2021 00:39, Daniel P. Smith wrote:
> The only difference between !CONFIG_XSM and CONFIG_XSM with !CONFIG_XSM_SILO and !CONFIG_XSM_FLASK
> is whether the XSM hooks in dummy.h are called as static inline functions or as function
> pointers to static functions. As such this commit,
> * eliminates CONFIG_XSM
> * introduces CONFIG_XSM_EVTCHN_LABELING as replacement for enabling event channel labels
> * makes CONFIG_XSM_SILO AND CONFIG_XSM_FLASK default to no
>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
> xen/common/Kconfig | 55 ++++-----
> xen/include/xen/sched.h | 2 +-
> xen/include/xsm/xsm-core.h | 26 ----
> xen/include/xsm/xsm.h | 8 --
> xen/xsm/Makefile | 4 +-
> xen/xsm/dummy.c | 4 +-
> xen/{include => }/xsm/dummy.h | 220 ++++++++++++++++------------------
> xen/xsm/silo.c | 17 +--
> xen/xsm/xsm_core.c | 4 -
> 9 files changed, 142 insertions(+), 198 deletions(-)
> rename xen/{include => }/xsm/dummy.h (63%)
>
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index 0ddd18e11a..203ad7ea23 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -197,22 +197,33 @@ config XENOPROF
>
> If unsure, say Y.
>
> -config XSM
> - bool "Xen Security Modules support"
> - default ARM
> - ---help---
> - Enables the security framework known as Xen Security Modules which
> - allows administrators fine-grained control over a Xen domain and
> - its capabilities by defining permissible interactions between domains,
> - the hypervisor itself, and related resources such as memory and
> - devices.
> +menu "Xen Security Modules"
>
> - If unsure, say N.
> +choice
> + prompt "Default XSM module"
> + default XSM_SILO_DEFAULT if XSM_SILO && ARM
> + default XSM_FLASK_DEFAULT if XSM_FLASK
> + default XSM_SILO_DEFAULT if XSM_SILO
> + default XSM_DUMMY_DEFAULT
> + config XSM_DUMMY_DEFAULT
> + bool "Match non-XSM behavior"

There is no non-XSM behaviour any more.

Is it time to rename Dummy to "traditional dom0-all-powerful" or
something suitable?

> + config XSM_FLASK_DEFAULT
> + bool "FLux Advanced Security Kernel" if XSM_FLASK
> + config XSM_SILO_DEFAULT
> + bool "SILO" if XSM_SILO
> +endchoice
> +
> +config XSM_EVTCHN_LABELING
> + bool "Enables security labeling of event channels"
> + default n
> + ---help---
> + This enables an XSM module to label and enforce access control over
> + event channels.

Please use help rather than ---help--- for new options (its changed in
upstream Kconfig).  The indentation of the help message wants to be one
tab, then two spaces.  (Yes, sadly...)

> config XSM_FLASK
> - def_bool y
> + def_bool n
> prompt "FLux Advanced Security Kernel support"
> - depends on XSM
> + select XSM_EVTCHN_LABELING
> ---help---
> Enables FLASK (FLux Advanced Security Kernel) as the access control
> mechanism used by the XSM framework. This provides a mandatory access
> @@ -250,9 +261,8 @@ config XSM_FLASK_POLICY
> If unsure, say Y.
>
> config XSM_SILO
> - def_bool y
> + def_bool n

I'm not sure we want to alter the FLASK/SILO defaults.  SILO in
particular is mandatory on ARM, and without it, you're in a security
unsupported configuration.

~Andrew
Re: [PATCH 3/6] xsm: enabling xsm to always be included [ In reply to ]
On 18.06.2021 01:39, Daniel P. Smith wrote:
> The only difference between !CONFIG_XSM and CONFIG_XSM with !CONFIG_XSM_SILO and !CONFIG_XSM_FLASK
> is whether the XSM hooks in dummy.h are called as static inline functions or as function
> pointers to static functions. As such this commit,
> * eliminates CONFIG_XSM

Following from what Andrew has said (including him mentioning your
changing of certain Kconfig option defaults), I'm not convinced this is
a good move. This still ought to serve as the overall XSM-yes-or-no
setting. If internally you make said two variants match in behavior,
that's a different thing.

> * introduces CONFIG_XSM_EVTCHN_LABELING as replacement for enabling event channel labels

Is this mode needed as separate functionality at all? Nothing defines
XSM_NEED_GENERIC_EVTCHN_SSID anywhere. _If_ XSM went away as a separate
setting, then imo this one should go away as well.

> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -197,22 +197,33 @@ config XENOPROF
>
> If unsure, say Y.
>
> -config XSM
> - bool "Xen Security Modules support"
> - default ARM
> - ---help---
> - Enables the security framework known as Xen Security Modules which
> - allows administrators fine-grained control over a Xen domain and
> - its capabilities by defining permissible interactions between domains,
> - the hypervisor itself, and related resources such as memory and
> - devices.
> +menu "Xen Security Modules"
>
> - If unsure, say N.
> +choice
> + prompt "Default XSM module"
> + default XSM_SILO_DEFAULT if XSM_SILO && ARM
> + default XSM_FLASK_DEFAULT if XSM_FLASK
> + default XSM_SILO_DEFAULT if XSM_SILO
> + default XSM_DUMMY_DEFAULT
> + config XSM_DUMMY_DEFAULT
> + bool "Match non-XSM behavior"
> + config XSM_FLASK_DEFAULT
> + bool "FLux Advanced Security Kernel" if XSM_FLASK
> + config XSM_SILO_DEFAULT
> + bool "SILO" if XSM_SILO
> +endchoice

This did live after the individual options it depends on for a reason,
and you don't say anywhere why you need to move it up. The way you
have it, with the default command line kconfig tool, users will be
presented with dependent options before having chosen the settings of
the dependency ones. That's because this tool, to a degree, moves
linearly through the options it has parsed.

> @@ -261,25 +271,12 @@ config XSM_SILO
>
> If unsure, say Y.
>
> -choice
> - prompt "Default XSM implementation"
> - depends on XSM
> - default XSM_SILO_DEFAULT if XSM_SILO && ARM
> - default XSM_FLASK_DEFAULT if XSM_FLASK
> - default XSM_SILO_DEFAULT if XSM_SILO
> - default XSM_DUMMY_DEFAULT
> - config XSM_DUMMY_DEFAULT
> - bool "Match non-XSM behavior"
> - config XSM_FLASK_DEFAULT
> - bool "FLux Advanced Security Kernel" if XSM_FLASK
> - config XSM_SILO_DEFAULT
> - bool "SILO" if XSM_SILO
> -endchoice
> +endmenu
>
> config LATE_HWDOM
> bool "Dedicated hardware domain"
> default n
> - depends on XSM && X86
> + depends on XSM_FLASK && X86

I don't think this is a compatible change. I'm not going to exclude that
this is how it was meant, but as it stands LATE_HWDOM right now doesn't
really require FLASK, and could e.g. also go with SILO or dummy. If you
_mean_ to change this, then your description needs to say so (and ideally
it would then be split out, so - if this is actually a bug - it could
also be backported).

Jan
Re: [PATCH 3/6] xsm: enabling xsm to always be included [ In reply to ]
On 6/18/21 7:53 AM, Andrew Cooper wrote:
> On 18/06/2021 00:39, Daniel P. Smith wrote:
>> The only difference between !CONFIG_XSM and CONFIG_XSM with !CONFIG_XSM_SILO and !CONFIG_XSM_FLASK
>> is whether the XSM hooks in dummy.h are called as static inline functions or as function
>> pointers to static functions. As such this commit,
>> * eliminates CONFIG_XSM
>> * introduces CONFIG_XSM_EVTCHN_LABELING as replacement for enabling event channel labels
>> * makes CONFIG_XSM_SILO AND CONFIG_XSM_FLASK default to no
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
>> xen/common/Kconfig | 55 ++++-----
>> xen/include/xen/sched.h | 2 +-
>> xen/include/xsm/xsm-core.h | 26 ----
>> xen/include/xsm/xsm.h | 8 --
>> xen/xsm/Makefile | 4 +-
>> xen/xsm/dummy.c | 4 +-
>> xen/{include => }/xsm/dummy.h | 220 ++++++++++++++++------------------
>> xen/xsm/silo.c | 17 +--
>> xen/xsm/xsm_core.c | 4 -
>> 9 files changed, 142 insertions(+), 198 deletions(-)
>> rename xen/{include => }/xsm/dummy.h (63%)
>>
>> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
>> index 0ddd18e11a..203ad7ea23 100644
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -197,22 +197,33 @@ config XENOPROF
>>
>> If unsure, say Y.
>>
>> -config XSM
>> - bool "Xen Security Modules support"
>> - default ARM
>> - ---help---
>> - Enables the security framework known as Xen Security Modules which
>> - allows administrators fine-grained control over a Xen domain and
>> - its capabilities by defining permissible interactions between domains,
>> - the hypervisor itself, and related resources such as memory and
>> - devices.
>> +menu "Xen Security Modules"
>>
>> - If unsure, say N.
>> +choice
>> + prompt "Default XSM module"
>> + default XSM_SILO_DEFAULT if XSM_SILO && ARM
>> + default XSM_FLASK_DEFAULT if XSM_FLASK
>> + default XSM_SILO_DEFAULT if XSM_SILO
>> + default XSM_DUMMY_DEFAULT
>> + config XSM_DUMMY_DEFAULT
>> + bool "Match non-XSM behavior"
>
> There is no non-XSM behaviour any more.
>
> Is it time to rename Dummy to "traditional dom0-all-powerful" or

Well, I left as dummy since that is what it has been known by thus far
and additionally the subsequent patch set was going to rename this to
XSM_ROLES/"XSM Role-based Access Control" For the intermediate time, I
can change the wording to reflect the correct state.

>> + config XSM_FLASK_DEFAULT
>> + bool "FLux Advanced Security Kernel" if XSM_FLASK
>> + config XSM_SILO_DEFAULT
>> + bool "SILO" if XSM_SILO
>> +endchoice
>> +
>> +config XSM_EVTCHN_LABELING
>> + bool "Enables security labeling of event channels"
>> + default n
>> + ---help---
>> + This enables an XSM module to label and enforce access control over
>> + event channels.
>
> Please use help rather than ---help--- for new options (its changed in
> upstream Kconfig).  The indentation of the help message wants to be one
> tab, then two spaces.  (Yes, sadly...)

ack

>> config XSM_FLASK
>> - def_bool y
>> + def_bool n
>> prompt "FLux Advanced Security Kernel support"
>> - depends on XSM
>> + select XSM_EVTCHN_LABELING
>> ---help---
>> Enables FLASK (FLux Advanced Security Kernel) as the access control
>> mechanism used by the XSM framework. This provides a mandatory access
>> @@ -250,9 +261,8 @@ config XSM_FLASK_POLICY
>> If unsure, say Y.
>>
>> config XSM_SILO
>> - def_bool y
>> + def_bool n
>
> I'm not sure we want to alter the FLASK/SILO defaults.  SILO in
> particular is mandatory on ARM, and without it, you're in a security
> unsupported configuration.
The intent here is the default is the classic dom0 configuration. What
if I did,

def bool n
def bool y if ARM

v/r
dps
Re: [PATCH 3/6] xsm: enabling xsm to always be included [ In reply to ]
On 6/18/21 8:26 AM, Jan Beulich wrote:
> On 18.06.2021 01:39, Daniel P. Smith wrote:
>> The only difference between !CONFIG_XSM and CONFIG_XSM with !CONFIG_XSM_SILO and !CONFIG_XSM_FLASK
>> is whether the XSM hooks in dummy.h are called as static inline functions or as function
>> pointers to static functions. As such this commit,
>> * eliminates CONFIG_XSM
>
> Following from what Andrew has said (including him mentioning your
> changing of certain Kconfig option defaults), I'm not convinced this is
> a good move. This still ought to serve as the overall XSM-yes-or-no
> setting. If internally you make said two variants match in behavior,
> that's a different thing.

Apologies that I did not express this clearly. What I was attempting to
say is the fact of the matter is that there is no logical behavior
difference between "XSM no" and "XSM yes with dummy policy". The only
difference is the mechanics of how the dummy functions get called.
Specifically via macro magic the dummy functions are either flipped into
static inline declarations that are then included into the code where
they are invoked or the macro magic has them ending up in the dummy.c
XSM module where they are wrapped in macro generated functions that are
set as the functions in the dummy xsm_ops structure. Thus it is always
the same logic being invoked, it is just mechanics of how you get to the
logic.


>> * introduces CONFIG_XSM_EVTCHN_LABELING as replacement for enabling event channel labels
>
> Is this mode needed as separate functionality at all? Nothing defines
> XSM_NEED_GENERIC_EVTCHN_SSID anywhere. _If_ XSM went away as a separate
> setting, then imo this one should go away as well.
>
>> --- a/xen/common/Kconfig
>> +++ b/xen/common/Kconfig
>> @@ -197,22 +197,33 @@ config XENOPROF
>>
>> If unsure, say Y.
>>
>> -config XSM
>> - bool "Xen Security Modules support"
>> - default ARM
>> - ---help---
>> - Enables the security framework known as Xen Security Modules which
>> - allows administrators fine-grained control over a Xen domain and
>> - its capabilities by defining permissible interactions between domains,
>> - the hypervisor itself, and related resources such as memory and
>> - devices.
>> +menu "Xen Security Modules"
>>
>> - If unsure, say N.
>> +choice
>> + prompt "Default XSM module"
>> + default XSM_SILO_DEFAULT if XSM_SILO && ARM
>> + default XSM_FLASK_DEFAULT if XSM_FLASK
>> + default XSM_SILO_DEFAULT if XSM_SILO
>> + default XSM_DUMMY_DEFAULT
>> + config XSM_DUMMY_DEFAULT
>> + bool "Match non-XSM behavior"
>> + config XSM_FLASK_DEFAULT
>> + bool "FLux Advanced Security Kernel" if XSM_FLASK
>> + config XSM_SILO_DEFAULT
>> + bool "SILO" if XSM_SILO
>> +endchoice
>
> This did live after the individual options it depends on for a reason,
> and you don't say anywhere why you need to move it up. The way you
> have it, with the default command line kconfig tool, users will be
> presented with dependent options before having chosen the settings of
> the dependency ones. That's because this tool, to a degree, moves
> linearly through the options it has parsed.

Yes, this is specifically why I moved it up. Clearly we have different
approaches to how we like to interact with configurations, which is not
bad thing. I personally found it awkward the other way but can easily
move it back.

>> @@ -261,25 +271,12 @@ config XSM_SILO
>>
>> If unsure, say Y.
>>
>> -choice
>> - prompt "Default XSM implementation"
>> - depends on XSM
>> - default XSM_SILO_DEFAULT if XSM_SILO && ARM
>> - default XSM_FLASK_DEFAULT if XSM_FLASK
>> - default XSM_SILO_DEFAULT if XSM_SILO
>> - default XSM_DUMMY_DEFAULT
>> - config XSM_DUMMY_DEFAULT
>> - bool "Match non-XSM behavior"
>> - config XSM_FLASK_DEFAULT
>> - bool "FLux Advanced Security Kernel" if XSM_FLASK
>> - config XSM_SILO_DEFAULT
>> - bool "SILO" if XSM_SILO
>> -endchoice
>> +endmenu
>>
>> config LATE_HWDOM
>> bool "Dedicated hardware domain"
>> default n
>> - depends on XSM && X86
>> + depends on XSM_FLASK && X86
>
> I don't think this is a compatible change. I'm not going to exclude that
> this is how it was meant, but as it stands LATE_HWDOM right now doesn't
> really require FLASK, and could e.g. also go with SILO or dummy. If you
> _mean_ to change this, then your description needs to say so (and ideally
> it would then be split out, so - if this is actually a bug - it could
> also be backported).

Actually this is the root cause that started all of this work. If you
want to get technical, LATE_HWDOM does not rely on XSM at all. The issue
is that you cannot use it as it was originally intended, to run Xen
without a classic dom0 while still having all existing capabilities.
Specifically the hardware domain does not have the ability to assign the
pass-through devices for which it is in control. This is were Flask
comes in to enable assigning specific privileges to labels and then
constructing domains with those labels, In particular it grants the
ability to do pass-through assignment to the label assigned to the
hardware domain. With the upcoming XSM-Roles patch set, these privileges
are assigned to roles and it becomes possible to assign the necessary
roles to the hardware domain.

v/r,
dps
Re: [PATCH 3/6] xsm: enabling xsm to always be included [ In reply to ]
On 18/06/2021 13:26, Jan Beulich wrote:
> On 18.06.2021 01:39, Daniel P. Smith wrote:
>> The only difference between !CONFIG_XSM and CONFIG_XSM with !CONFIG_XSM_SILO and !CONFIG_XSM_FLASK
>> is whether the XSM hooks in dummy.h are called as static inline functions or as function
>> pointers to static functions. As such this commit,
>> * eliminates CONFIG_XSM
> Following from what Andrew has said (including him mentioning your
> changing of certain Kconfig option defaults), I'm not convinced this is
> a good move. This still ought to serve as the overall XSM-yes-or-no
> setting. If internally you make said two variants match in behavior,
> that's a different thing.

I firmly disagree. There is no such thing as !XSM even in staging right now.

All over Xen, we have calls to xsm_*() functions which, even in the !XSM
case, contain a non-trivial security policy.

The fact that under the hood, XSM vs !XSM creates two very different
implementations of "the dom0-all-powerful model" is an error needing
correcting, as it contributes a massive quantity of code complexity.

This series of Daniel's takes steps to make the code match reality, and
getting rid of CONFIG_XSM is absolutely the right thing to do.  XSM is
never actually absent from a build of Xen, even if you choose CONFIG_XSM=n.


I do think that the thing we currently call XSM_DUMMY wants renaming to
indicate that it is "the dom0-all-powerful" security model, and I think
that wants doing as part of this series.  Name suggestions welcome.

~Andrew
Re: [PATCH 3/6] xsm: enabling xsm to always be included [ In reply to ]
On 18.06.2021 18:35, Daniel P. Smith wrote:
> On 6/18/21 7:53 AM, Andrew Cooper wrote:
>> On 18/06/2021 00:39, Daniel P. Smith wrote:
>>> @@ -250,9 +261,8 @@ config XSM_FLASK_POLICY
>>> If unsure, say Y.
>>>
>>> config XSM_SILO
>>> - def_bool y
>>> + def_bool n
>>
>> I'm not sure we want to alter the FLASK/SILO defaults.  SILO in
>> particular is mandatory on ARM, and without it, you're in a security
>> unsupported configuration.
> The intent here is the default is the classic dom0 configuration. What
> if I did,
>
> def bool n
> def bool y if ARM

Besides it needing to be with the order of the two lines flipped, if
Arm requires XSM_SILO, then I think it would better "select" it.

Jan
Re: [PATCH 3/6] xsm: enabling xsm to always be included [ In reply to ]
On 18.06.2021 22:27, Daniel P. Smith wrote:
> On 6/18/21 8:26 AM, Jan Beulich wrote:
>> On 18.06.2021 01:39, Daniel P. Smith wrote:
>>> The only difference between !CONFIG_XSM and CONFIG_XSM with !CONFIG_XSM_SILO and !CONFIG_XSM_FLASK
>>> is whether the XSM hooks in dummy.h are called as static inline functions or as function
>>> pointers to static functions. As such this commit,
>>> * eliminates CONFIG_XSM
>>
>> Following from what Andrew has said (including him mentioning your
>> changing of certain Kconfig option defaults), I'm not convinced this is
>> a good move. This still ought to serve as the overall XSM-yes-or-no
>> setting. If internally you make said two variants match in behavior,
>> that's a different thing.
>
> Apologies that I did not express this clearly. What I was attempting to
> say is the fact of the matter is that there is no logical behavior
> difference between "XSM no" and "XSM yes with dummy policy". The only
> difference is the mechanics of how the dummy functions get called.
> Specifically via macro magic the dummy functions are either flipped into
> static inline declarations that are then included into the code where
> they are invoked or the macro magic has them ending up in the dummy.c
> XSM module where they are wrapped in macro generated functions that are
> set as the functions in the dummy xsm_ops structure. Thus it is always
> the same logic being invoked, it is just mechanics of how you get to the
> logic.

That's what I understood, really. What I dislike is the inline functions
going away in what we currently call !XSM.

>>> * introduces CONFIG_XSM_EVTCHN_LABELING as replacement for enabling event channel labels
>>
>> Is this mode needed as separate functionality at all? Nothing defines
>> XSM_NEED_GENERIC_EVTCHN_SSID anywhere. _If_ XSM went away as a separate
>> setting, then imo this one should go away as well.
>>
>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -197,22 +197,33 @@ config XENOPROF
>>>
>>> If unsure, say Y.
>>>
>>> -config XSM
>>> - bool "Xen Security Modules support"
>>> - default ARM
>>> - ---help---
>>> - Enables the security framework known as Xen Security Modules which
>>> - allows administrators fine-grained control over a Xen domain and
>>> - its capabilities by defining permissible interactions between domains,
>>> - the hypervisor itself, and related resources such as memory and
>>> - devices.
>>> +menu "Xen Security Modules"
>>>
>>> - If unsure, say N.
>>> +choice
>>> + prompt "Default XSM module"
>>> + default XSM_SILO_DEFAULT if XSM_SILO && ARM
>>> + default XSM_FLASK_DEFAULT if XSM_FLASK
>>> + default XSM_SILO_DEFAULT if XSM_SILO
>>> + default XSM_DUMMY_DEFAULT
>>> + config XSM_DUMMY_DEFAULT
>>> + bool "Match non-XSM behavior"
>>> + config XSM_FLASK_DEFAULT
>>> + bool "FLux Advanced Security Kernel" if XSM_FLASK
>>> + config XSM_SILO_DEFAULT
>>> + bool "SILO" if XSM_SILO
>>> +endchoice
>>
>> This did live after the individual options it depends on for a reason,
>> and you don't say anywhere why you need to move it up. The way you
>> have it, with the default command line kconfig tool, users will be
>> presented with dependent options before having chosen the settings of
>> the dependency ones. That's because this tool, to a degree, moves
>> linearly through the options it has parsed.
>
> Yes, this is specifically why I moved it up. Clearly we have different
> approaches to how we like to interact with configurations, which is not
> bad thing. I personally found it awkward the other way but can easily
> move it back.

I'm having a hard time seeing how presenting dependent options ahead
of their dependencies can be a good thing: The user will have made
their "choice" here, while the availability of the individual settings
then may change in case the depended upon options change from their
defaults upon the user reacting to those prompts.

>>> @@ -261,25 +271,12 @@ config XSM_SILO
>>>
>>> If unsure, say Y.
>>>
>>> -choice
>>> - prompt "Default XSM implementation"
>>> - depends on XSM
>>> - default XSM_SILO_DEFAULT if XSM_SILO && ARM
>>> - default XSM_FLASK_DEFAULT if XSM_FLASK
>>> - default XSM_SILO_DEFAULT if XSM_SILO
>>> - default XSM_DUMMY_DEFAULT
>>> - config XSM_DUMMY_DEFAULT
>>> - bool "Match non-XSM behavior"
>>> - config XSM_FLASK_DEFAULT
>>> - bool "FLux Advanced Security Kernel" if XSM_FLASK
>>> - config XSM_SILO_DEFAULT
>>> - bool "SILO" if XSM_SILO
>>> -endchoice
>>> +endmenu
>>>
>>> config LATE_HWDOM
>>> bool "Dedicated hardware domain"
>>> default n
>>> - depends on XSM && X86
>>> + depends on XSM_FLASK && X86
>>
>> I don't think this is a compatible change. I'm not going to exclude that
>> this is how it was meant, but as it stands LATE_HWDOM right now doesn't
>> really require FLASK, and could e.g. also go with SILO or dummy. If you
>> _mean_ to change this, then your description needs to say so (and ideally
>> it would then be split out, so - if this is actually a bug - it could
>> also be backported).
>
> Actually this is the root cause that started all of this work. If you
> want to get technical, LATE_HWDOM does not rely on XSM at all. The issue
> is that you cannot use it as it was originally intended, to run Xen
> without a classic dom0 while still having all existing capabilities.
> Specifically the hardware domain does not have the ability to assign the
> pass-through devices for which it is in control. This is were Flask
> comes in to enable assigning specific privileges to labels and then
> constructing domains with those labels, In particular it grants the
> ability to do pass-through assignment to the label assigned to the
> hardware domain. With the upcoming XSM-Roles patch set, these privileges
> are assigned to roles and it becomes possible to assign the necessary
> roles to the hardware domain.

In which case this needs spelling out in the description, to justify
the change in behavior (which I'm not sure I really follow / agree with
just yet).

Jan
Re: [PATCH 3/6] xsm: enabling xsm to always be included [ In reply to ]
On 18.06.2021 23:20, Andrew Cooper wrote:
> On 18/06/2021 13:26, Jan Beulich wrote:
>> On 18.06.2021 01:39, Daniel P. Smith wrote:
>>> The only difference between !CONFIG_XSM and CONFIG_XSM with !CONFIG_XSM_SILO and !CONFIG_XSM_FLASK
>>> is whether the XSM hooks in dummy.h are called as static inline functions or as function
>>> pointers to static functions. As such this commit,
>>> * eliminates CONFIG_XSM
>> Following from what Andrew has said (including him mentioning your
>> changing of certain Kconfig option defaults), I'm not convinced this is
>> a good move. This still ought to serve as the overall XSM-yes-or-no
>> setting. If internally you make said two variants match in behavior,
>> that's a different thing.
>
> I firmly disagree. There is no such thing as !XSM even in staging right now.
>
> All over Xen, we have calls to xsm_*() functions which, even in the !XSM
> case, contain a non-trivial security policy.

Compared with the full-fledged XSM, I view the present xsm_default_action()
as sufficiently trivial. The inline wrappers of it really only exist to
allow #ifdef-ary at all the use sites to be avoided, and for the code to
act like before XSM got introduced. Whether you call this !XSM or
XSM_HWDOM_ALL_POWERFUL is secondary to me.

> The fact that under the hood, XSM vs !XSM creates two very different
> implementations of "the dom0-all-powerful model" is an error needing
> correcting, as it contributes a massive quantity of code complexity.
>
> This series of Daniel's takes steps to make the code match reality, and
> getting rid of CONFIG_XSM is absolutely the right thing to do.  XSM is
> never actually absent from a build of Xen, even if you choose CONFIG_XSM=n.

As said, what you discuss is just how to call the child. What I point out
as undesirable is the going away of the inline functions, replaced by real
function calls (not indirect ones thanks to alternatives patching, but
still not clearly on par with the current model in terms of overhead).

Jan
Re: [PATCH 3/6] xsm: enabling xsm to always be included [ In reply to ]
On 21/06/2021 07:58, Jan Beulich wrote:
> On 18.06.2021 22:27, Daniel P. Smith wrote:
>> On 6/18/21 8:26 AM, Jan Beulich wrote:
>>> On 18.06.2021 01:39, Daniel P. Smith wrote:
>>>> The only difference between !CONFIG_XSM and CONFIG_XSM with !CONFIG_XSM_SILO and !CONFIG_XSM_FLASK
>>>> is whether the XSM hooks in dummy.h are called as static inline functions or as function
>>>> pointers to static functions. As such this commit,
>>>> * eliminates CONFIG_XSM
>>> Following from what Andrew has said (including him mentioning your
>>> changing of certain Kconfig option defaults), I'm not convinced this is
>>> a good move. This still ought to serve as the overall XSM-yes-or-no
>>> setting. If internally you make said two variants match in behavior,
>>> that's a different thing.
>> Apologies that I did not express this clearly. What I was attempting to
>> say is the fact of the matter is that there is no logical behavior
>> difference between "XSM no" and "XSM yes with dummy policy". The only
>> difference is the mechanics of how the dummy functions get called.
>> Specifically via macro magic the dummy functions are either flipped into
>> static inline declarations that are then included into the code where
>> they are invoked or the macro magic has them ending up in the dummy.c
>> XSM module where they are wrapped in macro generated functions that are
>> set as the functions in the dummy xsm_ops structure. Thus it is always
>> the same logic being invoked, it is just mechanics of how you get to the
>> logic.
> That's what I understood, really. What I dislike is the inline functions
> going away in what we currently call !XSM.

I'm sorry, but this is an unreasonable objection.

The mess used to create the status quo *is* the majority reason why
fixing/developing XSM is so hard, and why the code is so obfuscated.  To
prove this point, how many people on this email thread realise that
calls using XSM_HOOK offer 0 security under xsm_default_action()?

Having xsm_default_action() forced inline isn't obviously the right move
in the first place, and I doubt that you could even measure a
performance difference for using real function calls.

Even if there is a marginal performance difference, and I doubt that
there is, performance is far less important than de-obfuscating the code
and fixing our various security mechanisms to be first-class supported
citizens.

~Andrew
Re: [PATCH 3/6] xsm: enabling xsm to always be included [ In reply to ]
On 21.06.2021 12:41, Andrew Cooper wrote:
> On 21/06/2021 07:58, Jan Beulich wrote:
>> On 18.06.2021 22:27, Daniel P. Smith wrote:
>>> On 6/18/21 8:26 AM, Jan Beulich wrote:
>>>> On 18.06.2021 01:39, Daniel P. Smith wrote:
>>>>> The only difference between !CONFIG_XSM and CONFIG_XSM with !CONFIG_XSM_SILO and !CONFIG_XSM_FLASK
>>>>> is whether the XSM hooks in dummy.h are called as static inline functions or as function
>>>>> pointers to static functions. As such this commit,
>>>>> * eliminates CONFIG_XSM
>>>> Following from what Andrew has said (including him mentioning your
>>>> changing of certain Kconfig option defaults), I'm not convinced this is
>>>> a good move. This still ought to serve as the overall XSM-yes-or-no
>>>> setting. If internally you make said two variants match in behavior,
>>>> that's a different thing.
>>> Apologies that I did not express this clearly. What I was attempting to
>>> say is the fact of the matter is that there is no logical behavior
>>> difference between "XSM no" and "XSM yes with dummy policy". The only
>>> difference is the mechanics of how the dummy functions get called.
>>> Specifically via macro magic the dummy functions are either flipped into
>>> static inline declarations that are then included into the code where
>>> they are invoked or the macro magic has them ending up in the dummy.c
>>> XSM module where they are wrapped in macro generated functions that are
>>> set as the functions in the dummy xsm_ops structure. Thus it is always
>>> the same logic being invoked, it is just mechanics of how you get to the
>>> logic.
>> That's what I understood, really. What I dislike is the inline functions
>> going away in what we currently call !XSM.
>
> I'm sorry, but this is an unreasonable objection.
>
> The mess used to create the status quo *is* the majority reason why
> fixing/developing XSM is so hard, and why the code is so obfuscated.  To
> prove this point, how many people on this email thread realise that
> calls using XSM_HOOK offer 0 security under xsm_default_action()?
>
> Having xsm_default_action() forced inline isn't obviously the right move
> in the first place, and I doubt that you could even measure a
> performance difference for using real function calls.
>
> Even if there is a marginal performance difference, and I doubt that
> there is, performance is far less important than de-obfuscating the code
> and fixing our various security mechanisms to be first-class supported
> citizens.

What I don't understand from all you say is why you think that having
an as-if-no-XSM build configuration, without any way to switch to an
alternative model (i.e. the XSM=n that we have right now), is a bad
thing. I don't mind the XSM=y case getting improved, but I don't see
(yet) why it is a good thing to force this onto everyone.

Jan
Re: [PATCH 3/6] xsm: enabling xsm to always be included [ In reply to ]
On 6/21/21 2:53 AM, Jan Beulich wrote:
> On 18.06.2021 18:35, Daniel P. Smith wrote:
>> On 6/18/21 7:53 AM, Andrew Cooper wrote:
>>> On 18/06/2021 00:39, Daniel P. Smith wrote:
>>>> @@ -250,9 +261,8 @@ config XSM_FLASK_POLICY
>>>> If unsure, say Y.
>>>>
>>>> config XSM_SILO
>>>> - def_bool y
>>>> + def_bool n
>>>
>>> I'm not sure we want to alter the FLASK/SILO defaults.  SILO in
>>> particular is mandatory on ARM, and without it, you're in a security
>>> unsupported configuration.
>> The intent here is the default is the classic dom0 configuration. What
>> if I did,
>>
>> def bool n
>> def bool y if ARM
>
> Besides it needing to be with the order of the two lines flipped, if
> Arm requires XSM_SILO, then I think it would better "select" it.


Ack, I realized that as I fixed it for the upcoming v2.

Correct me if I am wrong but if you do a "select" that means you are
forcing the user to always have SILO built in, i.e. that makes it so the
option cannot be disabled. There may be users who would prefer to only
have Flask enabled on ARM and those users would not be able to turn SILO
off.

v/r,
dps
Re: [PATCH 3/6] xsm: enabling xsm to always be included [ In reply to ]
On 24.06.2021 19:18, Daniel P. Smith wrote:
>
>
> On 6/21/21 2:53 AM, Jan Beulich wrote:
>> On 18.06.2021 18:35, Daniel P. Smith wrote:
>>> On 6/18/21 7:53 AM, Andrew Cooper wrote:
>>>> On 18/06/2021 00:39, Daniel P. Smith wrote:
>>>>> @@ -250,9 +261,8 @@ config XSM_FLASK_POLICY
>>>>> If unsure, say Y.
>>>>>
>>>>> config XSM_SILO
>>>>> - def_bool y
>>>>> + def_bool n
>>>>
>>>> I'm not sure we want to alter the FLASK/SILO defaults.  SILO in
>>>> particular is mandatory on ARM, and without it, you're in a security
>>>> unsupported configuration.
>>> The intent here is the default is the classic dom0 configuration. What
>>> if I did,
>>>
>>> def bool n
>>> def bool y if ARM
>>
>> Besides it needing to be with the order of the two lines flipped, if
>> Arm requires XSM_SILO, then I think it would better "select" it.
>
>
> Ack, I realized that as I fixed it for the upcoming v2.
>
> Correct me if I am wrong but if you do a "select" that means you are
> forcing the user to always have SILO built in, i.e. that makes it so the
> option cannot be disabled. There may be users who would prefer to only
> have Flask enabled on ARM and those users would not be able to turn SILO
> off.

Yes, you're right. Problem is the (imo) malformed entry, which makes
it that I couldn't see the presence of a prompt anymore in the context
above. Well-formed (imo; I might also say "consistently formatted")
entries with a prompt ought to look like (taking your change into
account already, leaving aside whether that's really what we want)

config XSM_SILO
bool "SILO support"
default y if ARM
default n

Whether "depends" precedes or follows "default" is a less clear cut.

def_bool imo would better be used only for prompt-less entries.

Jan