| Message ID | 20220523154024.1947-1-dpsmith@apertussolutions.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xsm: refactor and optimize policy loading | expand |
On 23.05.2022 17:40, Daniel P. Smith wrote: > --- a/xen/xsm/xsm_core.c > +++ b/xen/xsm/xsm_core.c > @@ -55,19 +55,35 @@ static enum xsm_bootparam __initdata xsm_bootparam = > XSM_BOOTPARAM_DUMMY; > #endif > > +static bool __initdata init_policy = > +#ifdef CONFIG_XSM_FLASK_DEFAULT > + true; > +#else > + false; > +#endif Simply IS_ENABLED(CONFIG_XSM_FLASK_DEFAULT) without any #ifdef-ary? > @@ -148,11 +156,11 @@ int __init xsm_multiboot_init( > > printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n"); > > - if ( XSM_MAGIC ) > + if ( init_policy && XSM_MAGIC ) > { > ret = xsm_multiboot_policy_init(module_map, mbi, &policy_buffer, > &policy_size); > - if ( ret ) > + if ( ret != 0 ) Nit: Stray change? > @@ -36,10 +36,17 @@ int __init xsm_multiboot_policy_init( > { > int i; > module_t *mod = (module_t *)__va(mbi->mods_addr); > - int rc = 0; > + int rc = -ENOENT; I'm afraid I can't easily convince myself that this and the other -ENOENT is really relevant to this change and/or not breaking anything which currently does work (i.e. not entirely benign). Please can you extend the description accordingly or split off this adjustment? > @@ -79,7 +87,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; I don't think we want a cast here, especially not when it discards "const". Instead the local variables' types want adjusting in xsm_{multiboot,dt}_init() as well as the types of the respective parameters of xsm_{multiboot,dt}_policy_init(). > + *policy_size = xsm_flask_init_policy_size; > return 0; > + } > +#else > + /* No policy was loaded */ > + return -ENOENT; > +#endif I think this is easier to read if you put the braces there unconditionally and have the #if / #else inside. And if you wanted to I think you could get away without any #else then. Jan
On Mon, May 23, 2022 at 11:40 AM Daniel P. Smith <dpsmith@apertussolutions.com> wrote: > > 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 does two things, it moves all policy initialization logic under the > xsm_XXXX_policy_init() functions and introduces the init_policy flag. The > init_policy flag will be set based on which enforcing policy is selected and > gates whether the boot modules should be checked for a policy file. I can see the use of init_policy to skip the search. (I'm not the biggest fan of the name, need_policy/uses_policy/has_policy?, but that's not a big deal). That part seems fine. I don't care for the movement of `policy_buffer = xsm_flask_init_policy;` since it replaces the single location with two locations. I prefer leaving the built-in policy fallback in xsm_core_init since it is multiboot/devicetree agnostic. i.e. the boot-method specific code passes a policy if it finds one, and xsm_core_init can fallback to the built-in policy if none is supplied. Since a built-in policy is flask specific, it could potentially be pushed down in flask_init. Is there a need for the xsm_flask_init_policy movement I am missing? Regards, Jason
On 5/24/22 11:50, Jan Beulich wrote: > On 23.05.2022 17:40, Daniel P. Smith wrote: >> --- a/xen/xsm/xsm_core.c >> +++ b/xen/xsm/xsm_core.c >> @@ -55,19 +55,35 @@ static enum xsm_bootparam __initdata xsm_bootparam = >> XSM_BOOTPARAM_DUMMY; >> #endif >> >> +static bool __initdata init_policy = >> +#ifdef CONFIG_XSM_FLASK_DEFAULT >> + true; >> +#else >> + false; >> +#endif > > Simply IS_ENABLED(CONFIG_XSM_FLASK_DEFAULT) without any #ifdef-ary? Ack, didn't even think of that. Thanks! >> @@ -148,11 +156,11 @@ int __init xsm_multiboot_init( >> >> printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n"); >> >> - if ( XSM_MAGIC ) >> + if ( init_policy && XSM_MAGIC ) >> { >> ret = xsm_multiboot_policy_init(module_map, mbi, &policy_buffer, >> &policy_size); >> - if ( ret ) >> + if ( ret != 0 ) > > Nit: Stray change? Yep, I twiddled with the if statement while testing/debugging all the permutations of enabled policies, default policy, built-in policy, and cmdline selected policy. Will clean up. >> @@ -36,10 +36,17 @@ int __init xsm_multiboot_policy_init( >> { >> int i; >> module_t *mod = (module_t *)__va(mbi->mods_addr); >> - int rc = 0; >> + int rc = -ENOENT; > > I'm afraid I can't easily convince myself that this and the other > -ENOENT is really relevant to this change and/or not breaking > anything which currently does work (i.e. not entirely benign). > Please can you extend the description accordingly or split off > this adjustment? Let me expand the commit explanation, and you can let me know how much of this detail you would like to see in the commit message. When enabling CONFIG_XSM_FLASK today, the XSM_MAGIC variable becomes defined as a non-zero value. This results in xsm_XXXX_policy_init() to be called regardless of the XSM policy selection, either through the respective CONFIG_XSM_XXXXX_DEFAULT flags or through the cmdline parameter. Additionally, the concept of policy initialization is split between xsm_XXXX_policy_init() and xsm_core_init(), with the latter being where the built-in policy would be selected. This forces xsm_XXXX_policy_init() to have to return successful for configurations where no policy loading was necessary. It also means that the situation where selecting XSM flask, with no built-in policy, and failure to provide a policy via MB/DT relies on the security server to fault when it is unable to load a policy. What this commit does is moves all policy buffer initialization to xsm_XXXX_policy_init(). As the init function, it should only return success (0) if it was able to initialize the policy buffer with either the built-in policy or a policy loaded from MB/DT. The second half of this commit corrects the logical flow so that the policy buffer initialization only occurs for XSM policy modules that consume a policy buffer, e.g. flask. I would note the opening of the commit message notes dom0less and hyperlaunch because I came across this while finalizing the first hyperlaunch patch series focused on fully integrating hyperlaunch's bootmodule structure, which will also will enable commonality with dom0less' bootmodule structure, into x86 MB1/MB2/EFI startup paths. I figured this change should just go alone and not with hyperlaunch bootmodule work. >> @@ -79,7 +87,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; > > I don't think we want a cast here, especially not when it discards > "const". Instead the local variables' types want adjusting in > xsm_{multiboot,dt}_init() as well as the types of the respective > parameters of xsm_{multiboot,dt}_policy_init(). True, will fix. >> + *policy_size = xsm_flask_init_policy_size; >> return 0; >> + } >> +#else >> + /* No policy was loaded */ >> + return -ENOENT; >> +#endif > > I think this is easier to read if you put the braces there > unconditionally and have the #if / #else inside. And if you wanted > to I think you could get away without any #else then. Ack. v/r, dps
On 5/24/22 12:17, Jason Andryuk wrote: > On Mon, May 23, 2022 at 11:40 AM Daniel P. Smith > <dpsmith@apertussolutions.com> wrote: >> >> 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 does two things, it moves all policy initialization logic under the >> xsm_XXXX_policy_init() functions and introduces the init_policy flag. The >> init_policy flag will be set based on which enforcing policy is selected and >> gates whether the boot modules should be checked for a policy file. > > I can see the use of init_policy to skip the search. (I'm not the > biggest fan of the name, need_policy/uses_policy/has_policy?, but > that's not a big deal). That part seems fine. Yep, I went through that and several other variations trying to find the one that would "read" best at the places it was set and checked. If anyone has a strong stance for a preferred naming, I have no objections. > I don't care for the movement of `policy_buffer = > xsm_flask_init_policy;` since it replaces the single location with two > locations. I prefer leaving the built-in policy fallback in > xsm_core_init since it is multiboot/devicetree agnostic. i.e. the > boot-method specific code passes a policy if it finds one, and > xsm_core_init can fallback to the built-in policy if none is supplied. > Since a built-in policy is flask specific, it could potentially be > pushed down in flask_init. > > Is there a need for the xsm_flask_init_policy movement I am missing? I would be willing to bet that the de-duplication is the reason it was initially done this way. But as I explained in the response to Jan, doing so means that xsm_XXXX_policy_init() will have to return success even if it did not successfully initialize the policy buffer. I considered making a static inline function to point the policy buffer at the built-in policy, but quickly walked back from it. The reason being is that it is not any real logic duplications, just two lines of variable setting. IMHO a single repeat of setting a pair of variables is better than the bifurcating of the policy buffer initialization. v/r, dps
On 24.05.2022 19:42, Daniel P. Smith wrote: > On 5/24/22 11:50, Jan Beulich wrote: >> On 23.05.2022 17:40, Daniel P. Smith wrote: >>> @@ -36,10 +36,17 @@ int __init xsm_multiboot_policy_init( >>> { >>> int i; >>> module_t *mod = (module_t *)__va(mbi->mods_addr); >>> - int rc = 0; >>> + int rc = -ENOENT; >> >> I'm afraid I can't easily convince myself that this and the other >> -ENOENT is really relevant to this change and/or not breaking >> anything which currently does work (i.e. not entirely benign). >> Please can you extend the description accordingly or split off >> this adjustment? > > Let me expand the commit explanation, and you can let me know how much > of this detail you would like to see in the commit message. > > When enabling CONFIG_XSM_FLASK today, the XSM_MAGIC variable becomes > defined as a non-zero value. This results in xsm_XXXX_policy_init() to > be called regardless of the XSM policy selection, either through the > respective CONFIG_XSM_XXXXX_DEFAULT flags or through the cmdline > parameter. Additionally, the concept of policy initialization is split > between xsm_XXXX_policy_init() and xsm_core_init(), with the latter > being where the built-in policy would be selected. This forces > xsm_XXXX_policy_init() to have to return successful for configurations > where no policy loading was necessary. It also means that the situation > where selecting XSM flask, with no built-in policy, and failure to > provide a policy via MB/DT relies on the security server to fault when > it is unable to load a policy. > > What this commit does is moves all policy buffer initialization to > xsm_XXXX_policy_init(). As the init function, it should only return > success (0) if it was able to initialize the policy buffer with either > the built-in policy or a policy loaded from MB/DT. The second half of > this commit corrects the logical flow so that the policy buffer > initialization only occurs for XSM policy modules that consume a policy > buffer, e.g. flask. I'm afraid this doesn't clarify anything for me wrt the addition of -ENOENT. There's no case of returning -ENOENT right now afaics (and there's no case of you dealing with the -ENOENT anywhere in your changes, so there would look to be an overall change in behavior as viewed from the outside, i.e. the callers of xsm_{multiboot,dt}_init()). If that's wrong for some reason (and yes, it looks at least questionable on the surface), that's what wants spelling out imo. A question then might be whether that's not a separate change, potentially even a fix which may want to be considered for backport. Of course otoh the sole caller of xsm_multiboot_init() ignores the return value altogether, and the sole caller of xsm_dt_init() only cares for the specific value of 1. This in turn looks at least questionable to me. Jan
On 5/25/22 02:37, Jan Beulich wrote: > On 24.05.2022 19:42, Daniel P. Smith wrote: >> On 5/24/22 11:50, Jan Beulich wrote: >>> On 23.05.2022 17:40, Daniel P. Smith wrote: >>>> @@ -36,10 +36,17 @@ int __init xsm_multiboot_policy_init( >>>> { >>>> int i; >>>> module_t *mod = (module_t *)__va(mbi->mods_addr); >>>> - int rc = 0; >>>> + int rc = -ENOENT; >>> >>> I'm afraid I can't easily convince myself that this and the other >>> -ENOENT is really relevant to this change and/or not breaking >>> anything which currently does work (i.e. not entirely benign). >>> Please can you extend the description accordingly or split off >>> this adjustment? >> >> Let me expand the commit explanation, and you can let me know how much >> of this detail you would like to see in the commit message. >> >> When enabling CONFIG_XSM_FLASK today, the XSM_MAGIC variable becomes >> defined as a non-zero value. This results in xsm_XXXX_policy_init() to >> be called regardless of the XSM policy selection, either through the >> respective CONFIG_XSM_XXXXX_DEFAULT flags or through the cmdline >> parameter. Additionally, the concept of policy initialization is split >> between xsm_XXXX_policy_init() and xsm_core_init(), with the latter >> being where the built-in policy would be selected. This forces >> xsm_XXXX_policy_init() to have to return successful for configurations >> where no policy loading was necessary. It also means that the situation >> where selecting XSM flask, with no built-in policy, and failure to >> provide a policy via MB/DT relies on the security server to fault when >> it is unable to load a policy. >> >> What this commit does is moves all policy buffer initialization to >> xsm_XXXX_policy_init(). As the init function, it should only return >> success (0) if it was able to initialize the policy buffer with either >> the built-in policy or a policy loaded from MB/DT. The second half of >> this commit corrects the logical flow so that the policy buffer >> initialization only occurs for XSM policy modules that consume a policy >> buffer, e.g. flask. > > I'm afraid this doesn't clarify anything for me wrt the addition of > -ENOENT. There's no case of returning -ENOENT right now afaics (and > there's no case of you dealing with the -ENOENT anywhere in your > changes, so there would look to be an overall change in behavior as > viewed from the outside, i.e. the callers of xsm_{multiboot,dt}_init()). > If that's wrong for some reason (and yes, it looks at least questionable > on the surface), that's what wants spelling out imo. A question then > might be whether that's not a separate change, potentially even a fix > which may want to be considered for backport. Of course otoh the sole > caller of xsm_multiboot_init() ignores the return value altogether, > and the sole caller of xsm_dt_init() only cares for the specific value > of 1. This in turn looks at least questionable to me. Okay, let me change the view a bit. The existing behavior is for xsm_{multiboot,dt}_init() to return 0 if they did not locate a policy file for initializing the policy buffer. It did so because it expected later that xsm_core_init() would just set it to the built-in policy. BUT, it also served the purpose of succeeding if it were called when either the dummy or SILO, neither of which needs the policy buffer initialized, is the enforcing policy. This change starts with moving the lines that set the policy buffer to the built-in policy into xsm_{multiboot,dt}_init() respectively and only return success if it was able to populate the policy buffer. In other words, if there is not a built-in policy and a policy file was not able to be loaded, it returns -NOENT to represent it was not able to find a policy file. This change makes these functions do as their name suggests, to initialize the policy buffer either from MB or DT with a fallback to the built-in policy otherwise fail. Upon making the function behave correctly, it exposes a valid set of conditions that under the current behavior succeeds but will fail under the correct behavior for xsm_{multiboot,dt}_init(). With flask enabled in the build but not the built-in policy and either dummy or SILO is the enforcing policy, then xsm_{multiboot,dt}_init() will be called and fail. This is where the second half of the change comes into effect, which is to introduce a gating that will only attempt to initialize the policy buffer if the enforcing XSM policy requires a policy file. With this gating in place, under the above set of conditions xsm_{multiboot,dt}_init() will not be called and XSM initialization will succeed as it should. Now to your question of whether these changes could be split and given a focused explanation in their respective commits. Yes, I can split it into two separate commits. While the gating of the call to xsm_{multiboot,dt}_init() is an independent change, the change to xsm_{multiboot,dt}_init() itself is not and must proceed after the gating change. This means it is possible to backport the first commit, gating, independently. If the desire is to backport the second commit, xsm_{multiboot,dt}_init() behavior, then it would require both commits to be backported. I hope this helps better clarify my reasoning and if you would like to see the changes split how I highlighted, just let me know. v/r, dps
On 25.05.2022 16:11, Daniel P. Smith wrote: > On 5/25/22 02:37, Jan Beulich wrote: >> On 24.05.2022 19:42, Daniel P. Smith wrote: >>> On 5/24/22 11:50, Jan Beulich wrote: >>>> On 23.05.2022 17:40, Daniel P. Smith wrote: >>>>> @@ -36,10 +36,17 @@ int __init xsm_multiboot_policy_init( >>>>> { >>>>> int i; >>>>> module_t *mod = (module_t *)__va(mbi->mods_addr); >>>>> - int rc = 0; >>>>> + int rc = -ENOENT; >>>> >>>> I'm afraid I can't easily convince myself that this and the other >>>> -ENOENT is really relevant to this change and/or not breaking >>>> anything which currently does work (i.e. not entirely benign). >>>> Please can you extend the description accordingly or split off >>>> this adjustment? >>> >>> Let me expand the commit explanation, and you can let me know how much >>> of this detail you would like to see in the commit message. >>> >>> When enabling CONFIG_XSM_FLASK today, the XSM_MAGIC variable becomes >>> defined as a non-zero value. This results in xsm_XXXX_policy_init() to >>> be called regardless of the XSM policy selection, either through the >>> respective CONFIG_XSM_XXXXX_DEFAULT flags or through the cmdline >>> parameter. Additionally, the concept of policy initialization is split >>> between xsm_XXXX_policy_init() and xsm_core_init(), with the latter >>> being where the built-in policy would be selected. This forces >>> xsm_XXXX_policy_init() to have to return successful for configurations >>> where no policy loading was necessary. It also means that the situation >>> where selecting XSM flask, with no built-in policy, and failure to >>> provide a policy via MB/DT relies on the security server to fault when >>> it is unable to load a policy. >>> >>> What this commit does is moves all policy buffer initialization to >>> xsm_XXXX_policy_init(). As the init function, it should only return >>> success (0) if it was able to initialize the policy buffer with either >>> the built-in policy or a policy loaded from MB/DT. The second half of >>> this commit corrects the logical flow so that the policy buffer >>> initialization only occurs for XSM policy modules that consume a policy >>> buffer, e.g. flask. >> >> I'm afraid this doesn't clarify anything for me wrt the addition of >> -ENOENT. There's no case of returning -ENOENT right now afaics (and >> there's no case of you dealing with the -ENOENT anywhere in your >> changes, so there would look to be an overall change in behavior as >> viewed from the outside, i.e. the callers of xsm_{multiboot,dt}_init()). >> If that's wrong for some reason (and yes, it looks at least questionable >> on the surface), that's what wants spelling out imo. A question then >> might be whether that's not a separate change, potentially even a fix >> which may want to be considered for backport. Of course otoh the sole >> caller of xsm_multiboot_init() ignores the return value altogether, >> and the sole caller of xsm_dt_init() only cares for the specific value >> of 1. This in turn looks at least questionable to me. > > Okay, let me change the view a bit. > > The existing behavior is for xsm_{multiboot,dt}_init() to return 0 if > they did not locate a policy file for initializing the policy buffer. It > did so because it expected later that xsm_core_init() would just set it > to the built-in policy. BUT, it also served the purpose of succeeding if > it were called when either the dummy or SILO, neither of which needs the > policy buffer initialized, is the enforcing policy. > > This change starts with moving the lines that set the policy buffer to > the built-in policy into xsm_{multiboot,dt}_init() respectively and only > return success if it was able to populate the policy buffer. In other > words, if there is not a built-in policy and a policy file was not able > to be loaded, it returns -NOENT to represent it was not able to find a > policy file. This change makes these functions do as their name > suggests, to initialize the policy buffer either from MB or DT with a > fallback to the built-in policy otherwise fail. > > Upon making the function behave correctly, it exposes a valid set of > conditions that under the current behavior succeeds but will fail under > the correct behavior for xsm_{multiboot,dt}_init(). With flask enabled > in the build but not the built-in policy and either dummy or SILO is the > enforcing policy, then xsm_{multiboot,dt}_init() will be called and > fail. This is where the second half of the change comes into effect, > which is to introduce a gating that will only attempt to initialize the > policy buffer if the enforcing XSM policy requires a policy file. With > this gating in place, under the above set of conditions > xsm_{multiboot,dt}_init() will not be called and XSM initialization will > succeed as it should. > > Now to your question of whether these changes could be split and given a > focused explanation in their respective commits. Yes, I can split it > into two separate commits. While the gating of the call to > xsm_{multiboot,dt}_init() is an independent change, the change to > xsm_{multiboot,dt}_init() itself is not and must proceed after the > gating change. This means it is possible to backport the first commit, > gating, independently. If the desire is to backport the second commit, > xsm_{multiboot,dt}_init() behavior, then it would require both commits > to be backported. > > I hope this helps better clarify my reasoning and if you would like to > see the changes split how I highlighted, just let me know. I'd like to leave to you whether to split. All I'm after is that from the description it becomes clear what the (intended) effect of the added -ENOENT errors is, which didn't exist before. Now that we're about to start adopting some Misra-C rules, this may even need to extend to cover the case of so far missing error checks potentially being added up the call tree. Jan
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c index 2286a502e3..0dfc970283 100644 --- a/xen/xsm/xsm_core.c +++ b/xen/xsm/xsm_core.c @@ -55,19 +55,35 @@ static enum xsm_bootparam __initdata xsm_bootparam = XSM_BOOTPARAM_DUMMY; #endif +static bool __initdata init_policy = +#ifdef CONFIG_XSM_FLASK_DEFAULT + true; +#else + false; +#endif + static int __init cf_check parse_xsm_param(const char *s) { int rc = 0; if ( !strcmp(s, "dummy") ) + { xsm_bootparam = XSM_BOOTPARAM_DUMMY; + init_policy = false; + } #ifdef CONFIG_XSM_FLASK else if ( !strcmp(s, "flask") ) + { xsm_bootparam = XSM_BOOTPARAM_FLASK; + init_policy = true; + } #endif #ifdef CONFIG_XSM_SILO else if ( !strcmp(s, "silo") ) + { xsm_bootparam = XSM_BOOTPARAM_SILO; + init_policy = false; + } #endif else rc = -EINVAL; @@ -80,14 +96,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 @@ -148,11 +156,11 @@ int __init xsm_multiboot_init( printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n"); - if ( XSM_MAGIC ) + if ( init_policy && XSM_MAGIC ) { ret = xsm_multiboot_policy_init(module_map, mbi, &policy_buffer, &policy_size); - if ( ret ) + if ( ret != 0 ) { bootstrap_map(NULL); printk(XENLOG_ERR "Error %d initializing XSM policy\n", ret); @@ -176,7 +184,7 @@ int __init xsm_dt_init(void) printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n"); - if ( XSM_MAGIC ) + if ( init_policy && XSM_MAGIC ) { ret = xsm_dt_policy_init(&policy_buffer, &policy_size); if ( ret ) diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c index 8dafbc9381..0e32418999 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. @@ -36,10 +36,17 @@ int __init xsm_multiboot_policy_init( { 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 = (void *)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. @@ -61,6 +68,7 @@ int __init xsm_multiboot_policy_init( _policy_len,_policy_start); __clear_bit(i, module_map); + rc = 0; break; } @@ -79,7 +87,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 + /* No policy was loaded */ + return -ENOENT; +#endif paddr = mod->start; len = mod->size;
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 does two things, it moves all policy initialization logic under the xsm_XXXX_policy_init() functions and introduces the init_policy flag. The init_policy flag will be set based on which enforcing policy is selected and gates whether the boot modules should be checked for a policy file. Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> --- xen/xsm/xsm_core.c | 30 +++++++++++++++++++----------- xen/xsm/xsm_policy.c | 21 +++++++++++++++++++-- 2 files changed, 38 insertions(+), 13 deletions(-)