Re: [Xen-devel] [PATCH 17/17] xsm: add a default policy to .init.data

[Julien Grall <julien.grall@xxxxxxx>]

Hi Daniel,

On 24/06/16 18:34, Daniel De Graaf wrote:
> On 06/24/2016 12:50 PM, Konrad Rzeszutek Wilk wrote:
> > On Fri, Jun 24, 2016 at 05:30:32PM +0100, Julien Grall wrote:
> > > > diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
> > > > index 12fc3a9..eefd37c 100644
> > > > --- a/xen/xsm/flask/Makefile
> > > > +++ b/xen/xsm/flask/Makefile
> > > > @@ -27,6 +27,23 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
> > > >  $(AV_H_FILES): $(AV_H_DEPEND)
> > > >      $(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)
> > > >
> > > > +ifeq ($(CONFIG_XSM_POLICY),y)
> > > > +HAS_CHECKPOLICY := $(shell checkpolicy -h 2>&1 | grep -q xen &&
> > > > echo y || echo n)
> > > > +
> > > > +obj-$(HAS_CHECKPOLICY) += policy.o
> > > I would have expect a warning (if not an error) here to tell the user
> > > that
> > > checkpolicy is not available. Otherwise it may take some time to the
> > > user to
> > > understand why the policy is not loaded/present. Because if you
> > > enable XSM,
> > > you don't necessarily check which other options have been enabled by
> > > default.
> > Good point! And we should probably update the INSTALL document too to
> > mention
> > that you need checkpoint tool!
> The dependency on checkpolicy is called out in the Kconfig item that
> enables this option.
AFAICT, the dependency is only called out for CONFIG_XSM_POLICY. 
However, this option is enabled by default if CONFIG_XSM is selected.
User may not read what was enabled by default because of CONFIG_XSM 
selected.
In any case, silently disable an option is a bad idea and will likely 
cause trouble in the future if we ever decide to enable XSM by default. 
Another use case would be, a user post his .config on the ML asking why 
the policy is not loaded.
Regards,

--
Julien Grall

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