[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3] xsm: add a default policy to .init.data
On Thu, Jun 30, 2016 at 10:01:18AM -0400, Daniel De Graaf wrote: > On 06/30/2016 09:45 AM, Konrad Rzeszutek Wilk wrote: > > On Wed, Jun 29, 2016 at 11:09:01AM -0400, Daniel De Graaf wrote: > > > This adds a Kconfig option and support for including the XSM policy from > > > tools/flask/policy in the hypervisor so that the bootloader does not > > > need to provide a policy to get sane behavior from an XSM-enabled > > > hypervisor. The policy provided by the bootloader, if present, will > > > override the built-in policy. > > > > > > The XSM policy is not moved out of tools because that remains the > > > primary location for installing and configuring the policy. > > > > > > Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> > > > --- > > > > > > Changes from v2 (dropped acks and reviewed-by): > > > - Drop linker script changes, use python binary-to-C file script > > > - Make the config option always include the policy if selected > > > - Note the new conditional dependency on checkpolicy in INSTALL > > > > I liked the previous patch of putting in it in __init section. > > Is that something this patch could do? Ah, n/m. I see that > > the python script generates the binary with __init! > > > > Secondly I was wondering why the suggestion I had - which was to check > > of the 'checkpolicy' availability - and if not found - then > > hide the Kconfig option was not mentioned? > > At the moment, since FLASK is the only XSM implementation and it is not > enabled by default, if someone enables XSM they will need to generate a > security policy at some point: either during the hypervisor build or in > the tools build. Otherwise, they will need to disable FLASK at runtime > or (worst case) run without a policy at all, which means all domains can > act as dom0. If you plan to disable FLASK at runtime, it is better to > compile without XSM enabled so you don't pay the cost of the function > calls to the XSM hooks on (almost) every hypercall. > > Otherwise, I was planning to just change the default from always "y" to > "y" if checkpolicy was found and "n" if not. This would end up running > "checkpolicy -h" on every (recursive) Make invocation if placed in the > top-level Config.mk - I'm unsure if that would be a problem or not. We already do quite a lot of other checks in there. I am not sure if an extra one would be so bad. Do other maintainers care about it? > > > .. snip... > > > +sys.stdout.write("\n};\nconst int __init xsm_init_policy_size = %d;\n" % > > > policy_size) > > > diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c > > > index 8df1a3c..93c7d43 100644 > > > --- a/xen/xsm/xsm_core.c > > > +++ b/xen/xsm/xsm_core.c > > > @@ -36,6 +36,24 @@ static inline int verify(struct xsm_operations *ops) > > > return 0; > > > } > > > > > > +#ifdef CONFIG_XSM_POLICY > > > +extern char xsm_init_policy[]; > > > > > +extern int xsm_init_policy_size; > > > +#else > > > +#define xsm_init_policy 0 > > > +#endif > > > + > > > +static void __init xsm_policy_init(void) > > > +{ > > > +#ifdef CONFIG_XSM_POLICY > > > + if ( policy_size == 0 ) > > > + { > > > + policy_buffer = xsm_init_policy; > > > + policy_size = xsm_init_policy_size; > > > + } > > > +#endif > > > +} > > > + > > > > This all looks like it could go in a header file? > > Sure, I could move it to xsm.h. I thought it might be clearer to have the > assignment and the xfree() check in the same file. You are the maintainer of that code, so it is your call. Having externs in C code is not that great (from my perspective), so if you move all of this to a header file that should get easier? But as said - you are the maintainer and I am just voicing my preferences - and if the code looks "nicer" your way - by all means do it that way! I am not going to get offended :-) _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |