Re: [PATCH] tools: make flask utils build unconditional

From: Ian Campbell <ian.campbell@citrix.com>
To: Doug Goldstein <cardoe@cardoe.com>, Wei Liu <wei.liu2@citrix.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>,
	xen-devel@lists.xen.org, Ian Jackson <ian.jackson@eu.citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Subject: Re: [PATCH] tools: make flask utils build unconditional
Date: Tue, 5 Jan 2016 16:41:06 +0000	[thread overview]
Message-ID: <1452012066.13361.375.camel@citrix.com> (raw)
In-Reply-To: <568BF09D.8060007@cardoe.com>

On Tue, 2016-01-05 at 10:34 -0600, Doug Goldstein wrote:
> On 1/5/16 10:13 AM, Wei Liu wrote:
> > On Tue, Jan 05, 2016 at 03:36:21PM +0000, Ian Campbell wrote:
> > > On Tue, 2016-01-05 at 14:37 +0000, Ian Campbell wrote:
> > > > 
> > > > which on the basis of this discussion I wasn't expecting. I didn't
> > > > see this
> > > > new file on i686 or ARM*.
> > > > 
> > > > My baseline is from the last time I committed, which would be last
> > > > year, so
> > > > maybe something other than my current batch of patches has caused
> > > > this.
> > > > 
> > > > I'm going to drop this one for now and (hopefully) get the rest of
> > > > the
> > > > batch squared away. Afterwards I'll take another look (with a new
> > > > baseline
> > > > filelist), but if someone can explain it in the meantime that would
> > > > be
> > > > super.
> > > 
> > > So with a fresh basline I still see:
> > > 
> > > --- ../FILE_LIST.BASE.staging.x86_64    2016-01-05 14:50:32.000000000
> > > +0000
> > > +++ ../FILE_LIST.staging.x86_64 2016-01-05 15:11:15.000000000 +0000
> > > @@ -6,6 +6,7 @@
> > >  dist/install/boot/xen-4.7-unstable.gz
> > >  dist/install/boot/xen-4.gz
> > >  dist/install/boot/xen.gz
> > > +dist/install/boot/xenpolicy-4.7-unstable
> > >  dist/install/etc
> > >  dist/install/etc/bash_completion.d
> > >  dist/install/etc/bash_completion.d/xl.sh
> > > @@ -386,6 +387,12 @@
> > >  dist/install/usr/local/lib/xen/libexec
> > >  dist/install/usr/local/lib/xen/libexec/qemu-bridge-helper
> > >  dist/install/usr/local/sbin
> > > +dist/install/usr/local/sbin/flask-get-bool
> > > +dist/install/usr/local/sbin/flask-getenforce
> > > +dist/install/usr/local/sbin/flask-label-pci
> > > +dist/install/usr/local/sbin/flask-loadpolicy
> > > +dist/install/usr/local/sbin/flask-set-bool
> > > +dist/install/usr/local/sbin/flask-setenforce
> > >  dist/install/usr/local/sbin/gdbsx
> > >  dist/install/usr/local/sbin/gtracestat
> > >  dist/install/usr/local/sbin/gtraceview
> > > *** FILES DIFFER ***
> > > 
> > > On i686 and ARM* I only see the (expected) second hunk.
> > > 
> > > I think the i686 case is explainable by the lack of a hypervisor
> > > build
> > > there, but I'm unsure why ARM* and x86_64 should differ in this
> > > regard.
> > > 
> > > config/Tools.mk is y only on x86_64, not on the others, which
> > > obviously
> > > explains things, but the question is why only on x86_64 (I presume
> > > this has
> > > always been the case and it was previously masked, but I've not
> > > checked).
> > > 
> > > Ah, OK, I misread
> > > 
> > > AX_ARG_DEFAULT_ENABLE([xsmpolicy], [Disable XSM policy compilation])
> > > 
> > > as being default disable, actually the default is "enabled iff
> > > checkpolicy
> > > is installed" and it happens to be that it is only installed in my
> > > x86_64
> > > build env.
> > > 
> > > So, in the end I think Wei was correct and this change will now, in
> > > some
> > > circumstances, end up installing a /boot/xenpolicy-*.
> > > 
> > 
> > I don't think it is related to this patch. I see an xenpoilcy file
> > without this patch applied. As you said it only depends on availability
> > of checkpolicy (part of generic SELinux utils, not the ones we build).
> > 
> > That said, let me try to answer the following question.
> > 
> > > So the question is do we mind that?
> > > 
> > 
> > We might or might not. See below.
> > 
> > I once submitted a patch to grub that look into /boot and generate XSM
> > entries if there is policy file. The patch is not yet merged though.
> > 
> > Since there is no way at the moment to tell if xen.gz has flask
> > enabled,
> > my not yet upstreamed patch only matches the version number of xen.gz
> > and
> > xenpolicy. Installing xenpolicy when xen.gz is not flaks-capable will
> > make grub generate an XSM entry nonetheless, which makes no sense.
> > 
> > Of course all the above is based on the theory that my grub patch is
> > going to be upstreamed.
> > 
> > Things have changed since I first submitted that patch. Doug's Kconfig
> > work is good. With .config installed in suitable location we can make
> > grub grep for flask information in config, hence avoiding generating
> > wrong entries.  I think this is better solution as we don't need to use
> > version number to match xen.gz and xenpolicy. If we go down this route
> > we don't mind having random xenpolicy lying around in /boot.
> 
> So I submitted a patch to put the .config in /boot to have the ability
> to do this. I figured it needed to be in /boot because that's how the
> existing 20_linux_xen but there's some disagreement on the location. If
> we can resolve that I'll happily update the 20_linux_xen patch as well.
> 
> http://lists.xenproject.org/archives/html/xen-devel/2015-12/msg02369.html

It would be worth referring that thread back to this one too.

Ian.

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