From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH 7/7] tools/hotplug: add wrapper to start xenstored Date: Thu, 10 Sep 2015 16:11:08 +0100 Message-ID: References: <1418988333-5404-1-git-send-email-olaf@aepfle.de> <1418988333-5404-8-git-send-email-olaf@aepfle.de> <1420544515.28863.148.camel@citrix.com> <20150107094006.GC12049@aepfle.de> <20150910145330.GC1695@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: M A Young Cc: Olaf Hering , Wei Liu , Ian Campbell , Stefano Stabellini , Ian Jackson , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On Thu, Sep 10, 2015 at 4:01 PM, M A Young wrote: > On Thu, 10 Sep 2015, Wei Liu wrote: > >> On Thu, Sep 10, 2015 at 03:19:35PM +0100, George Dunlap wrote: >> > On Wed, Jan 7, 2015 at 9:40 AM, Olaf Hering wrote: >> > > On Tue, Jan 06, Ian Campbell wrote: >> > > >> > >> On Fri, 2014-12-19 at 12:25 +0100, Olaf Hering wrote: >> > >> > The shell wrapper in xenstored.service does not handle XENSTORE_TRACE. >> > >> > >> > >> > Create a separate wrapper script which is used in the sysv runlevel >> > >> > script and in systemd xenstored.service. It preserves existing >> > >> > behaviour by handling the XENSTORE_TRACE boolean. It also implements >> > >> > the handling of XENSTORED_ARGS=. This variable has to be added to >> > >> > sysconfig/xencommons. >> > >> >> > >> Why don't we just drop XENSTORED_* in favour of XENSTORED_ARGS (with an >> > >> example in the sysconfig file of enabling tracing if you like)? >> > > >> > > After having two weeks to think about this I came to the same >> > > conclusion. I think whatever the outcome is, the boolean should be >> > > removed. The sysconfig file should get a XENSTORED_ARGS="" along with a >> > > help text which mentions "-T /path" and "xenstored --help" to get other >> > > options because there is no man page. >> > > >> > >> Going to a wrapper script just to make some fairly uncommon debugging >> > >> option marginally more convenient seems like overkill to me, plus >> > >> XENSTORED_ARGS would allow for passing other useful options to >> > >> xenstored. >> > > >> > > If I recall correctly the point of the current 'sh -c "exec ..."' stunt >> > > was to expand the XENSTORE variable from the sysconfig file. But this >> > > approach leads to failures with SELinux because the socket passing does >> > > not work this way. Up to now I have not seen a success report for >> > > selinux+systemd+xenstored. Maybe its already somewhere in the other >> > > unread mails. >> > > >> > > In my cover letter I provided some possible ways to handle >> > > selinux+systemd+xenstored. Ideally the approach "Exec=/usr/bin/env >> > > $XENSTORED --no-fork $XENSTORED_ARGS" works because it means its >> > > possible to select a binary via the sysconfig file. But it also means >> > > the XENSTORE_TRACE boolean has to be removed in favour of the plain >> > > XENSTORED_ARGS= approach mentioned above. Finally this would avoid the >> > > need for a wrapper script. >> > > >> > > Hopefully someone with access to a SELinux enabled system will report >> > > which approach actually works. >> > >> > I can confirm that >> > >> > ExecStart=/bin/sh -c "exec $XENSTORED --no-fork $XENSTORED_ARGS" >> > >> > does not work on a CentOS7 system with selinux enabled, but that >> > >> > ExecStart=/usr/bin/env $XENSTORED --no-fork $XENSTORED_ARGS >> > >> >> And the difference between these two approaches is /usr/bin/env >> preserves envars while sh -c doesn't? > > /bin/sh doesn't give the right selinux permissions so xenstored doesn't > start. Presumably /usr/bin/env does. Or to be more precise, /bin/sh exec from that context doesn't allow xenstored to make the accept() system call, so you get a log full of these: type=PROCTITLE msg=audit(1441892166.173:22242): proctitle=2F7573722F7362696E2F78656E73746F726564002D2D6E6F2D666F726B type=AVC msg=audit(1441892166.173:22243): avc: denied { accept } for pid=615 comm="xenstored" path="/run/xenstored/socket" scontext=system_u:system_r:xenstored_t:s0 tcontext=system_u:system_r:initrc_t:s0 tclass=unix_stream_socket permissive=0 type=SYSCALL msg=audit(1441892166.173:22243): arch=c000003e syscall=43 success=no exit=-13 a0=3 a1=0 a2=0 a3=0 items=0 ppid=1 pid=615 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="xenstored" exe="/usr/sbin/xenstored" subj=system_u:system_r:xenstored_t:s0 key=(null) At least, I assume the denied {accept} means the accept system call... I haven't verified that syscall 43 is in fact accept. I assume that "env" since was designed to execute other programs, selinux on CentOS 7 has been programmed to retain the context. -George