All of lore.kernel.org
 help / color / mirror / Atom feed
* [v2 0/3] xsm: refactor and optimize policy loading
@ 2022-05-31  2:39 Daniel P. Smith
  2022-05-31  2:39 ` [v2 1/3] xsm: only search for a policy file when needed Daniel P. Smith
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Daniel P. Smith @ 2022-05-31  2:39 UTC (permalink / raw)
  To: xen-devel; +Cc: Daniel P. Smith, scott.davis, christopher.clark, jandryuk

This series was born out of some cleanup work done while crafting the
hyperlaunch boot modules patch series. The primary purpose of this series was
to stop walking all boot modules when it was not necessary, but the scope
creeped during review.

Changes in v2:
* changed init_policy to policy_file_required
* split the patch into a series
* corrected casting of policy buffer
* use IS_ENABLED() instead of #ifdef sequence
* moved #ifdef inside of braces for xsm_dt_policy_init()
* addressed lack of error handling of xsm{mb,dt}_init()

Daniel P. Smith (3):
  xsm: only search for a policy file when needed
  xsm: consolidate loading the policy buffer
  xsm: properly handle error from XSM init

 xen/arch/arm/setup.c  | 10 ++++----
 xen/arch/x86/setup.c  |  9 +++++--
 xen/include/xsm/xsm.h |  2 +-
 xen/xsm/xsm_core.c    | 56 +++++++++++++++++++++----------------------
 xen/xsm/xsm_policy.c  | 31 ++++++++++++++++++++----
 5 files changed, 68 insertions(+), 40 deletions(-)

-- 
2.20.1



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

* [v2 1/3] xsm: only search for a policy file when needed
  2022-05-31  2:39 [v2 0/3] xsm: refactor and optimize policy loading Daniel P. Smith
@ 2022-05-31  2:39 ` Daniel P. Smith
  2022-05-31  2:39 ` [v2 1/3] xsm: optimize policy loading Daniel P. Smith
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Smith @ 2022-05-31  2:39 UTC (permalink / raw)
  To: xen-devel, Daniel P. Smith
  Cc: scott.davis, christopher.clark, jandryuk, Daniel De Graaf

It is possible to select a few different build configurations that results in
the unnecessary walking of the boot module list looking for a policy module.
This specifically occurs when the flask policy is enabled but either the dummy
or the SILO policy is selected as the enforcing policy. This is not ideal for
configurations like hyperlaunch and dom0less when there could be a number of
modules to be walked or doing an unnecessary device tree lookup.

This patch introduces the policy_file_required flag for tracking when an XSM
policy module requires a policy file. Only when the policy_file_required flag
is set to true, will XSM search the boot modules for a policy file.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/xsm/xsm_core.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 2286a502e3..4a29ee9558 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -55,19 +55,31 @@ static enum xsm_bootparam __initdata xsm_bootparam =
     XSM_BOOTPARAM_DUMMY;
 #endif
 
+static bool __initdata policy_file_required =
+    IS_ENABLED(CONFIG_XSM_FLASK_DEFAULT);
+
 static int __init cf_check parse_xsm_param(const char *s)
 {
     int rc = 0;
 
     if ( !strcmp(s, "dummy") )
+    {
         xsm_bootparam = XSM_BOOTPARAM_DUMMY;
+        policy_file_required = false;
+    }
 #ifdef CONFIG_XSM_FLASK
     else if ( !strcmp(s, "flask") )
+    {
         xsm_bootparam = XSM_BOOTPARAM_FLASK;
+        policy_file_required = true;
+    }
 #endif
 #ifdef CONFIG_XSM_SILO
     else if ( !strcmp(s, "silo") )
+    {
         xsm_bootparam = XSM_BOOTPARAM_SILO;
+        policy_file_required = false;
+    }
 #endif
     else
         rc = -EINVAL;
@@ -148,7 +160,7 @@ int __init xsm_multiboot_init(
 
     printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
 
-    if ( XSM_MAGIC )
+    if ( policy_file_required && XSM_MAGIC )
     {
         ret = xsm_multiboot_policy_init(module_map, mbi, &policy_buffer,
                                         &policy_size);
@@ -176,7 +188,7 @@ int __init xsm_dt_init(void)
 
     printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
 
-    if ( XSM_MAGIC )
+    if ( policy_file_required && XSM_MAGIC )
     {
         ret = xsm_dt_policy_init(&policy_buffer, &policy_size);
         if ( ret )
-- 
2.20.1



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

* [v2 1/3] xsm: optimize policy loading
  2022-05-31  2:39 [v2 0/3] xsm: refactor and optimize policy loading Daniel P. Smith
  2022-05-31  2:39 ` [v2 1/3] xsm: only search for a policy file when needed Daniel P. Smith
@ 2022-05-31  2:39 ` Daniel P. Smith
  2022-05-31  2:39 ` [v2 2/3] xsm: consolidate loading the policy buffer Daniel P. Smith
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Smith @ 2022-05-31  2:39 UTC (permalink / raw)
  To: xen-devel, Daniel P. Smith
  Cc: scott.davis, christopher.clark, jandryuk, Daniel De Graaf

It is possible to select a few different build configurations that results in
the unnecessary walking of the boot module list looking for a policy module.
This specifically occurs when the flask policy is enabled but either the dummy
or the SILO policy is selected as the enforcing policy. This is not ideal for
configurations like hyperlaunch and dom0less when there could be a number of
modules to be walked or unnecessary device tree lookups

This patch introduces the policy_file_required flag for tracking when an XSM
policy module requires a policy file. Only when the policy_file_required flag
is set to true will XSM search the boot modules for a policy file.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/xsm/xsm_core.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 2286a502e3..4a29ee9558 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -55,19 +55,31 @@ static enum xsm_bootparam __initdata xsm_bootparam =
     XSM_BOOTPARAM_DUMMY;
 #endif
 
+static bool __initdata policy_file_required =
+    IS_ENABLED(CONFIG_XSM_FLASK_DEFAULT);
+
 static int __init cf_check parse_xsm_param(const char *s)
 {
     int rc = 0;
 
     if ( !strcmp(s, "dummy") )
+    {
         xsm_bootparam = XSM_BOOTPARAM_DUMMY;
+        policy_file_required = false;
+    }
 #ifdef CONFIG_XSM_FLASK
     else if ( !strcmp(s, "flask") )
+    {
         xsm_bootparam = XSM_BOOTPARAM_FLASK;
+        policy_file_required = true;
+    }
 #endif
 #ifdef CONFIG_XSM_SILO
     else if ( !strcmp(s, "silo") )
+    {
         xsm_bootparam = XSM_BOOTPARAM_SILO;
+        policy_file_required = false;
+    }
 #endif
     else
         rc = -EINVAL;
@@ -148,7 +160,7 @@ int __init xsm_multiboot_init(
 
     printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
 
-    if ( XSM_MAGIC )
+    if ( policy_file_required && XSM_MAGIC )
     {
         ret = xsm_multiboot_policy_init(module_map, mbi, &policy_buffer,
                                         &policy_size);
@@ -176,7 +188,7 @@ int __init xsm_dt_init(void)
 
     printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
 
-    if ( XSM_MAGIC )
+    if ( policy_file_required && XSM_MAGIC )
     {
         ret = xsm_dt_policy_init(&policy_buffer, &policy_size);
         if ( ret )
-- 
2.20.1



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

* [v2 2/3] xsm: consolidate loading the policy buffer
  2022-05-31  2:39 [v2 0/3] xsm: refactor and optimize policy loading Daniel P. Smith
  2022-05-31  2:39 ` [v2 1/3] xsm: only search for a policy file when needed Daniel P. Smith
  2022-05-31  2:39 ` [v2 1/3] xsm: optimize policy loading Daniel P. Smith
@ 2022-05-31  2:39 ` Daniel P. Smith
  2022-05-31  2:39 ` [v2 2/3] xsm: refactor policy loading Daniel P. Smith
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Smith @ 2022-05-31  2:39 UTC (permalink / raw)
  To: xen-devel, Daniel P. Smith
  Cc: scott.davis, christopher.clark, jandryuk, Daniel De Graaf

Previously, initializing the policy buffer was split between two functions,
xsm_{multiboot,dt}_policy_init() and xsm_core_init(). The latter for loading
the policy from boot modules and the former for falling back to built-in policy.

This patch moves all policy buffer initialization logic under the
xsm_{multiboot,dt}_policy_init() functions. It then ensures that an error
message is printed for every error condition that may occur in the functions.
With all policy buffer init contained and only called when the policy buffer
must be populated, the respective xsm_{mb,dt}_init() functions will panic if an
error occurs attempting to populate the policy buffer.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/include/xsm/xsm.h |  2 +-
 xen/xsm/xsm_core.c    | 18 +++---------------
 xen/xsm/xsm_policy.c  | 31 +++++++++++++++++++++++++++----
 3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 3e2b7fe3db..1676c261c9 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -775,7 +775,7 @@ int xsm_multiboot_init(
     unsigned long *module_map, const multiboot_info_t *mbi);
 int xsm_multiboot_policy_init(
     unsigned long *module_map, const multiboot_info_t *mbi,
-    void **policy_buffer, size_t *policy_size);
+    const unsigned char *policy_buffer[], size_t *policy_size);
 #endif
 
 #ifdef CONFIG_HAS_DEVICE_TREE
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 4a29ee9558..8f6c3de8a6 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -92,14 +92,6 @@ static int __init xsm_core_init(const void *policy_buffer, size_t policy_size)
 {
     const struct xsm_ops *ops = NULL;
 
-#ifdef CONFIG_XSM_FLASK_POLICY
-    if ( policy_size == 0 )
-    {
-        policy_buffer = xsm_flask_init_policy;
-        policy_size = xsm_flask_init_policy_size;
-    }
-#endif
-
     if ( xsm_ops_registered != XSM_OPS_UNREGISTERED )
     {
         printk(XENLOG_ERR
@@ -155,7 +147,7 @@ int __init xsm_multiboot_init(
     unsigned long *module_map, const multiboot_info_t *mbi)
 {
     int ret = 0;
-    void *policy_buffer = NULL;
+    const unsigned char *policy_buffer;
     size_t policy_size = 0;
 
     printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
@@ -167,8 +159,7 @@ int __init xsm_multiboot_init(
         if ( ret )
         {
             bootstrap_map(NULL);
-            printk(XENLOG_ERR "Error %d initializing XSM policy\n", ret);
-            return -EINVAL;
+            panic(XENLOG_ERR "Error %d initializing XSM policy\n", ret);
         }
     }
 
@@ -192,10 +183,7 @@ int __init xsm_dt_init(void)
     {
         ret = xsm_dt_policy_init(&policy_buffer, &policy_size);
         if ( ret )
-        {
-            printk(XENLOG_ERR "Error %d initializing XSM policy\n", ret);
-            return -EINVAL;
-        }
+            panic(XENLOG_ERR "Error %d initializing XSM policy\n", ret);
     }
 
     ret = xsm_core_init(policy_buffer, policy_size);
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index 8dafbc9381..6a4f769aec 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -8,7 +8,7 @@
  *  Contributors:
  *  Michael LeMay, <mdlemay@epoch.ncsc.mil>
  *  George Coker, <gscoker@alpha.ncsc.mil>
- *  
+ *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2,
  *  as published by the Free Software Foundation.
@@ -32,14 +32,21 @@
 #ifdef CONFIG_MULTIBOOT
 int __init xsm_multiboot_policy_init(
     unsigned long *module_map, const multiboot_info_t *mbi,
-    void **policy_buffer, size_t *policy_size)
+    const unsigned char *policy_buffer[], size_t *policy_size)
 {
     int i;
     module_t *mod = (module_t *)__va(mbi->mods_addr);
-    int rc = 0;
+    int rc = -ENOENT;
     u32 *_policy_start;
     unsigned long _policy_len;
 
+#ifdef CONFIG_XSM_FLASK_POLICY
+    /* Initially set to builtin policy, overriden if boot module is found. */
+    *policy_buffer = xsm_flask_init_policy;
+    *policy_size = xsm_flask_init_policy_size;
+    rc = 0;
+#endif
+
     /*
      * Try all modules and see whichever could be the binary policy.
      * Adjust module_map for the module that is the binary policy.
@@ -54,13 +61,14 @@ int __init xsm_multiboot_policy_init(
 
         if ( (xsm_magic_t)(*_policy_start) == XSM_MAGIC )
         {
-            *policy_buffer = _policy_start;
+            *policy_buffer = (unsigned char *)_policy_start;
             *policy_size = _policy_len;
 
             printk("Policy len %#lx, start at %p.\n",
                    _policy_len,_policy_start);
 
             __clear_bit(i, module_map);
+            rc = 0;
             break;
 
         }
@@ -68,6 +76,9 @@ int __init xsm_multiboot_policy_init(
         bootstrap_map(NULL);
     }
 
+    if ( rc == -ENOENT )
+        printk(XENLOG_ERR "xsm: Unable to locate policy file\n");
+
     return rc;
 }
 #endif
@@ -79,7 +90,16 @@ int __init xsm_dt_policy_init(void **policy_buffer, size_t *policy_size)
     paddr_t paddr, len;
 
     if ( !mod || !mod->size )
+    {
+#ifdef CONFIG_XSM_FLASK_POLICY
+        *policy_buffer = (void *)xsm_flask_init_policy;
+        *policy_size = xsm_flask_init_policy_size;
         return 0;
+#else
+        printk(XENLOG_ERR "xsm: Unable to locate policy file\n");
+        return -ENOENT;
+#endif
+    }
 
     paddr = mod->start;
     len = mod->size;
@@ -95,7 +115,10 @@ int __init xsm_dt_policy_init(void **policy_buffer, size_t *policy_size)
 
     *policy_buffer = xmalloc_bytes(len);
     if ( !*policy_buffer )
+    {
+        printk(XENLOG_ERR "xsm: Unable to allocate memory for XSM policy\n");
         return -ENOMEM;
+    }
 
     copy_from_paddr(*policy_buffer, paddr, len);
     *policy_size = len;
-- 
2.20.1



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

* [v2 2/3] xsm: refactor policy loading
  2022-05-31  2:39 [v2 0/3] xsm: refactor and optimize policy loading Daniel P. Smith
                   ` (2 preceding siblings ...)
  2022-05-31  2:39 ` [v2 2/3] xsm: consolidate loading the policy buffer Daniel P. Smith
@ 2022-05-31  2:39 ` Daniel P. Smith
  2022-05-31  2:39 ` [v2 3/3] xsm: properly handle error from XSM init Daniel P. Smith
  2022-05-31  8:25 ` [v2 0/3] xsm: refactor and optimize policy loading Jan Beulich
  5 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Smith @ 2022-05-31  2:39 UTC (permalink / raw)
  To: xen-devel, Daniel P. Smith
  Cc: scott.davis, christopher.clark, jandryuk, Daniel De Graaf

Previously initializing the policy buffer was split between two functions,
xsm_{multiboot,dt}_policy_init() and xsm_core_init(). The latter for loading
the policy from boot modules and the former for falling back to built-in policy.

This patch moves all policy buffer initialization logic under the
xsm_{multiboot,dt}_policy_init() functions. It then ensures that an error
message is printed for every error condition that may occur in the functions.
With all policy buffer init contained and only called when the policy buffer
must be populated, the respective xsm_{mb,dt}_init() functions will panic if an
error occurs attempting to populate the policy buffer.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/include/xsm/xsm.h |  2 +-
 xen/xsm/xsm_core.c    | 18 +++---------------
 xen/xsm/xsm_policy.c  | 31 +++++++++++++++++++++++++++----
 3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 3e2b7fe3db..1676c261c9 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -775,7 +775,7 @@ int xsm_multiboot_init(
     unsigned long *module_map, const multiboot_info_t *mbi);
 int xsm_multiboot_policy_init(
     unsigned long *module_map, const multiboot_info_t *mbi,
-    void **policy_buffer, size_t *policy_size);
+    const unsigned char *policy_buffer[], size_t *policy_size);
 #endif
 
 #ifdef CONFIG_HAS_DEVICE_TREE
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 4a29ee9558..8f6c3de8a6 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -92,14 +92,6 @@ static int __init xsm_core_init(const void *policy_buffer, size_t policy_size)
 {
     const struct xsm_ops *ops = NULL;
 
-#ifdef CONFIG_XSM_FLASK_POLICY
-    if ( policy_size == 0 )
-    {
-        policy_buffer = xsm_flask_init_policy;
-        policy_size = xsm_flask_init_policy_size;
-    }
-#endif
-
     if ( xsm_ops_registered != XSM_OPS_UNREGISTERED )
     {
         printk(XENLOG_ERR
@@ -155,7 +147,7 @@ int __init xsm_multiboot_init(
     unsigned long *module_map, const multiboot_info_t *mbi)
 {
     int ret = 0;
-    void *policy_buffer = NULL;
+    const unsigned char *policy_buffer;
     size_t policy_size = 0;
 
     printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
@@ -167,8 +159,7 @@ int __init xsm_multiboot_init(
         if ( ret )
         {
             bootstrap_map(NULL);
-            printk(XENLOG_ERR "Error %d initializing XSM policy\n", ret);
-            return -EINVAL;
+            panic(XENLOG_ERR "Error %d initializing XSM policy\n", ret);
         }
     }
 
@@ -192,10 +183,7 @@ int __init xsm_dt_init(void)
     {
         ret = xsm_dt_policy_init(&policy_buffer, &policy_size);
         if ( ret )
-        {
-            printk(XENLOG_ERR "Error %d initializing XSM policy\n", ret);
-            return -EINVAL;
-        }
+            panic(XENLOG_ERR "Error %d initializing XSM policy\n", ret);
     }
 
     ret = xsm_core_init(policy_buffer, policy_size);
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index 8dafbc9381..6a4f769aec 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -8,7 +8,7 @@
  *  Contributors:
  *  Michael LeMay, <mdlemay@epoch.ncsc.mil>
  *  George Coker, <gscoker@alpha.ncsc.mil>
- *  
+ *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2,
  *  as published by the Free Software Foundation.
@@ -32,14 +32,21 @@
 #ifdef CONFIG_MULTIBOOT
 int __init xsm_multiboot_policy_init(
     unsigned long *module_map, const multiboot_info_t *mbi,
-    void **policy_buffer, size_t *policy_size)
+    const unsigned char *policy_buffer[], size_t *policy_size)
 {
     int i;
     module_t *mod = (module_t *)__va(mbi->mods_addr);
-    int rc = 0;
+    int rc = -ENOENT;
     u32 *_policy_start;
     unsigned long _policy_len;
 
+#ifdef CONFIG_XSM_FLASK_POLICY
+    /* Initially set to builtin policy, overriden if boot module is found. */
+    *policy_buffer = xsm_flask_init_policy;
+    *policy_size = xsm_flask_init_policy_size;
+    rc = 0;
+#endif
+
     /*
      * Try all modules and see whichever could be the binary policy.
      * Adjust module_map for the module that is the binary policy.
@@ -54,13 +61,14 @@ int __init xsm_multiboot_policy_init(
 
         if ( (xsm_magic_t)(*_policy_start) == XSM_MAGIC )
         {
-            *policy_buffer = _policy_start;
+            *policy_buffer = (unsigned char *)_policy_start;
             *policy_size = _policy_len;
 
             printk("Policy len %#lx, start at %p.\n",
                    _policy_len,_policy_start);
 
             __clear_bit(i, module_map);
+            rc = 0;
             break;
 
         }
@@ -68,6 +76,9 @@ int __init xsm_multiboot_policy_init(
         bootstrap_map(NULL);
     }
 
+    if ( rc == -ENOENT )
+        printk(XENLOG_ERR "xsm: Unable to locate policy file\n");
+
     return rc;
 }
 #endif
@@ -79,7 +90,16 @@ int __init xsm_dt_policy_init(void **policy_buffer, size_t *policy_size)
     paddr_t paddr, len;
 
     if ( !mod || !mod->size )
+    {
+#ifdef CONFIG_XSM_FLASK_POLICY
+        *policy_buffer = (void *)xsm_flask_init_policy;
+        *policy_size = xsm_flask_init_policy_size;
         return 0;
+#else
+        printk(XENLOG_ERR "xsm: Unable to locate policy file\n");
+        return -ENOENT;
+#endif
+    }
 
     paddr = mod->start;
     len = mod->size;
@@ -95,7 +115,10 @@ int __init xsm_dt_policy_init(void **policy_buffer, size_t *policy_size)
 
     *policy_buffer = xmalloc_bytes(len);
     if ( !*policy_buffer )
+    {
+        printk(XENLOG_ERR "xsm: Unable to allocate memory for XSM policy\n");
         return -ENOMEM;
+    }
 
     copy_from_paddr(*policy_buffer, paddr, len);
     *policy_size = len;
-- 
2.20.1



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

* [v2 3/3] xsm: properly handle error from XSM init
  2022-05-31  2:39 [v2 0/3] xsm: refactor and optimize policy loading Daniel P. Smith
                   ` (3 preceding siblings ...)
  2022-05-31  2:39 ` [v2 2/3] xsm: refactor policy loading Daniel P. Smith
@ 2022-05-31  2:39 ` Daniel P. Smith
  2022-05-31  9:10   ` Bertrand Marquis
  2022-05-31  8:25 ` [v2 0/3] xsm: refactor and optimize policy loading Jan Beulich
  5 siblings, 1 reply; 9+ messages in thread
From: Daniel P. Smith @ 2022-05-31  2:39 UTC (permalink / raw)
  To: xen-devel, Volodymyr Babchuk, Wei Liu, Daniel P. Smith
  Cc: scott.davis, christopher.clark, jandryuk, Stefano Stabellini,
	Julien Grall, Bertrand Marquis, Jan Beulich, Andrew Cooper,
	Roger Pau Monné,
	Daniel De Graaf

This commit is to move towards providing a uniform interface across
architectures to initialize the XSM framework. Specifically, it provides a
common handling of initialization failure by providing the printing of a
warning message.

For Arm, xsm_dt_init() was tailored to have an Arm specific expansion of the
return values. This expansion added a value to reflect whether the security
supported XSM policy module was the enforcing policy module. This was then used
to determine if a warning message would be printed. Despite this expansion,
like x86, Arm does not address any XSM initialization errors that may have
occurred.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/arm/setup.c | 10 +++++-----
 xen/arch/x86/setup.c |  9 +++++++--
 xen/xsm/xsm_core.c   | 22 +++++++++++-----------
 3 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index ea1f5ee3d3..6bf71e1064 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -967,11 +967,11 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     tasklet_subsys_init();
 
-    if ( xsm_dt_init() != 1 )
-        warning_add("WARNING: SILO mode is not enabled.\n"
-                    "It has implications on the security of the system,\n"
-                    "unless the communications have been forbidden between\n"
-                    "untrusted domains.\n");
+    if ( xsm_dt_init() )
+        warning_add("WARNING: XSM failed to initialize.\n"
+                    "This has implications on the security of the system,\n"
+                    "as uncontrolled communications between trusted and\n"
+                    "untrusted domains may occur.\n");
 
     init_maintenance_interrupt();
     init_timer_interrupt();
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 53a73010e0..ed67b50c9d 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -24,6 +24,7 @@
 #include <xen/pfn.h>
 #include <xen/nodemask.h>
 #include <xen/virtual_region.h>
+#include <xen/warning.h>
 #include <xen/watchdog.h>
 #include <public/version.h>
 #ifdef CONFIG_COMPAT
@@ -1690,7 +1691,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     open_softirq(NEW_TLBFLUSH_CLOCK_PERIOD_SOFTIRQ, new_tlbflush_clock_period);
 
-    if ( opt_watchdog ) 
+    if ( opt_watchdog )
         nmi_watchdog = NMI_LOCAL_APIC;
 
     find_smp_config();
@@ -1700,7 +1701,11 @@ void __init noreturn __start_xen(unsigned long mbi_p)
     mmio_ro_ranges = rangeset_new(NULL, "r/o mmio ranges",
                                   RANGESETF_prettyprint_hex);
 
-    xsm_multiboot_init(module_map, mbi);
+    if ( xsm_multiboot_init(module_map, mbi) )
+        warning_add("WARNING: XSM failed to initialize.\n"
+                    "This has implications on the security of the system,\n"
+                    "as uncontrolled communications between trusted and\n"
+                    "untrusted domains may occur.\n");
 
     /*
      * IOMMU-related ACPI table parsing may require some of the system domains
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 8f6c3de8a6..6377895e1e 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -10,23 +10,17 @@
  *  as published by the Free Software Foundation.
  */
 
-#include <xen/init.h>
 #include <xen/errno.h>
+#include <xen/hypercall.h>
+#include <xen/init.h>
 #include <xen/lib.h>
 #include <xen/param.h>
-
-#include <xen/hypercall.h>
+#include <xen/warning.h>
 #include <xsm/xsm.h>
 
-#ifdef CONFIG_XSM
-
-#ifdef CONFIG_MULTIBOOT
 #include <asm/setup.h>
-#endif
 
-#ifdef CONFIG_HAS_DEVICE_TREE
-#include <asm/setup.h>
-#endif
+#ifdef CONFIG_XSM
 
 #define XSM_FRAMEWORK_VERSION    "1.0.1"
 
@@ -190,7 +184,13 @@ int __init xsm_dt_init(void)
 
     xfree(policy_buffer);
 
-    return ret ?: (xsm_bootparam == XSM_BOOTPARAM_SILO);
+    if ( xsm_bootparam != XSM_BOOTPARAM_SILO )
+        warning_add("WARNING: SILO mode is not enabled.\n"
+                    "It has implications on the security of the system,\n"
+                    "unless the communications have been forbidden between\n"
+                    "untrusted domains.\n");
+
+    return ret;
 }
 
 /**
-- 
2.20.1



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

* Re: [v2 0/3] xsm: refactor and optimize policy loading
  2022-05-31  2:39 [v2 0/3] xsm: refactor and optimize policy loading Daniel P. Smith
                   ` (4 preceding siblings ...)
  2022-05-31  2:39 ` [v2 3/3] xsm: properly handle error from XSM init Daniel P. Smith
@ 2022-05-31  8:25 ` Jan Beulich
  2022-05-31 10:38   ` Daniel P. Smith
  5 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2022-05-31  8:25 UTC (permalink / raw)
  To: Daniel P. Smith; +Cc: scott.davis, christopher.clark, jandryuk, xen-devel

On 31.05.2022 04:39, Daniel P. Smith wrote:
> This series was born out of some cleanup work done while crafting the
> hyperlaunch boot modules patch series. The primary purpose of this series was
> to stop walking all boot modules when it was not necessary, but the scope
> creeped during review.
> 
> Changes in v2:
> * changed init_policy to policy_file_required
> * split the patch into a series
> * corrected casting of policy buffer
> * use IS_ENABLED() instead of #ifdef sequence
> * moved #ifdef inside of braces for xsm_dt_policy_init()
> * addressed lack of error handling of xsm{mb,dt}_init()
> 
> Daniel P. Smith (3):
>   xsm: only search for a policy file when needed
>   xsm: consolidate loading the policy buffer
>   xsm: properly handle error from XSM init

The thread consists of 5 follow-ups, including two different 1/3 and two
different 2/3. This wants sorting and then perhaps (properly) resending.

Jan



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

* Re: [v2 3/3] xsm: properly handle error from XSM init
  2022-05-31  2:39 ` [v2 3/3] xsm: properly handle error from XSM init Daniel P. Smith
@ 2022-05-31  9:10   ` Bertrand Marquis
  0 siblings, 0 replies; 9+ messages in thread
From: Bertrand Marquis @ 2022-05-31  9:10 UTC (permalink / raw)
  To: Daniel P. Smith
  Cc: xen-devel, Volodymyr Babchuk, Wei Liu, scott.davis,
	christopher.clark, jandryuk, Stefano Stabellini, Julien Grall,
	Jan Beulich, Andrew Cooper, Roger Pau Monné,
	Daniel De Graaf

Hi Daniel,

> On 31 May 2022, at 03:39, Daniel P. Smith <dpsmith@apertussolutions.com> wrote:
> 
> This commit is to move towards providing a uniform interface across
> architectures to initialize the XSM framework. Specifically, it provides a
> common handling of initialization failure by providing the printing of a
> warning message.
> 
> For Arm, xsm_dt_init() was tailored to have an Arm specific expansion of the
> return values. This expansion added a value to reflect whether the security
> supported XSM policy module was the enforcing policy module. This was then used
> to determine if a warning message would be printed. Despite this expansion,
> like x86, Arm does not address any XSM initialization errors that may have
> occurred.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>

For the arm part:
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand




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

* Re: [v2 0/3] xsm: refactor and optimize policy loading
  2022-05-31  8:25 ` [v2 0/3] xsm: refactor and optimize policy loading Jan Beulich
@ 2022-05-31 10:38   ` Daniel P. Smith
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel P. Smith @ 2022-05-31 10:38 UTC (permalink / raw)
  To: Jan Beulich; +Cc: scott.davis, christopher.clark, jandryuk, xen-devel

On 5/31/22 04:25, Jan Beulich wrote:
> On 31.05.2022 04:39, Daniel P. Smith wrote:
>> This series was born out of some cleanup work done while crafting the
>> hyperlaunch boot modules patch series. The primary purpose of this series was
>> to stop walking all boot modules when it was not necessary, but the scope
>> creeped during review.
>>
>> Changes in v2:
>> * changed init_policy to policy_file_required
>> * split the patch into a series
>> * corrected casting of policy buffer
>> * use IS_ENABLED() instead of #ifdef sequence
>> * moved #ifdef inside of braces for xsm_dt_policy_init()
>> * addressed lack of error handling of xsm{mb,dt}_init()
>>
>> Daniel P. Smith (3):
>>   xsm: only search for a policy file when needed
>>   xsm: consolidate loading the policy buffer
>>   xsm: properly handle error from XSM init
> 
> The thread consists of 5 follow-ups, including two different 1/3 and two
> different 2/3. This wants sorting and then perhaps (properly) resending.

Yep, looks like I had stray copies from an internal review spin. My
apologies. Will resend clean version.

v/r,
dps



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

end of thread, other threads:[~2022-05-31 10:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-31  2:39 [v2 0/3] xsm: refactor and optimize policy loading Daniel P. Smith
2022-05-31  2:39 ` [v2 1/3] xsm: only search for a policy file when needed Daniel P. Smith
2022-05-31  2:39 ` [v2 1/3] xsm: optimize policy loading Daniel P. Smith
2022-05-31  2:39 ` [v2 2/3] xsm: consolidate loading the policy buffer Daniel P. Smith
2022-05-31  2:39 ` [v2 2/3] xsm: refactor policy loading Daniel P. Smith
2022-05-31  2:39 ` [v2 3/3] xsm: properly handle error from XSM init Daniel P. Smith
2022-05-31  9:10   ` Bertrand Marquis
2022-05-31  8:25 ` [v2 0/3] xsm: refactor and optimize policy loading Jan Beulich
2022-05-31 10:38   ` Daniel P. Smith

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.