[libvirt] [PATCH v3]LXC: Helper function for checking permission of dir when userns enabled
Daniel P. Berrange
berrange at redhat.com
Tue Oct 15 11:07:20 UTC 2013
On Tue, Oct 15, 2013 at 06:41:28PM +0800, Chen Hanxiao wrote:
> ping...
>
> > -----Original Message-----
> > From: libvir-list-bounces at redhat.com
> [mailto:libvir-list-bounces at redhat.com]
> > On Behalf Of Chen Hanxiao
> > Sent: Wednesday, October 09, 2013 6:03 PM
> > To: libvir-list at redhat.com
> > Subject: Re: [libvirt] [PATCH v3]LXC: Helper function for checking
> permission of
> > dir when userns enabled
> >
> > ping?
> >
> > > -----Original Message-----
> > > From: Chen Hanxiao [mailto:chenhanxiao at cn.fujitsu.com]
> > > Sent: Tuesday, September 10, 2013 4:18 PM
> > > To: libvir-list at redhat.com
> > > Cc: chenhanxiao at cn.fujitsu.com
> > > Subject: [libvirt][PATCH v3]LXC: Helper function for checking permission
> of dir
> > > when userns enabled
> > >
> > > From: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> > >
> > > If we enable userns, the process with uid/gid in idmap
> > > should have enough permission to access dir we provided
> > > for containers.
> > > Currently, the debug log is very implicit
> > > or misleading sometimes.
> > > This patch will help clarify this for us
> > > when using debug log or virsh.
> > >
> > > v2: syntax-check clean
> > >
> > > v3: reliable method for checking permission of dir
> > >
> > > Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> > > ---
> > > src/lxc/lxc_container.c | 88
> > > +++++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 88 insertions(+)
> > >
> > > diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> > > index 8abaea0..9a05e30 100644
> > > --- a/src/lxc/lxc_container.c
> > > +++ b/src/lxc/lxc_container.c
> > > @@ -110,6 +110,13 @@ struct __lxc_child_argv {
> > > int handshakefd;
> > > };
> > >
> > > +typedef struct __lxc_userns_DirPermCheck_argv
> > > lxc_userns_DirPermCheck_argv_t;
> > > +struct __lxc_userns_DirPermCheck_argv {
> > > + uid_t uid;
> > > + gid_t gid;
> > > + virDomainDefPtr vmDef;
> > > +};
> > > +
> > > static int lxcContainerMountFSBlock(virDomainFSDefPtr fs,
> > > const char *srcprefix);
> > >
> > > @@ -1829,6 +1836,84 @@ lxcNeedNetworkNamespace(virDomainDefPtr
> > def)
> > > return false;
> > > }
> > >
> > > +static
> > > +int lxcContainerCheckDirPermissionChild(void *argv)
> > > +{
> > > + size_t i;
> > > + lxc_userns_DirPermCheck_argv_t *args = argv;
> > > + uid_t uid = args->uid;
> > > + uid_t gid = args->gid;
> > > + virDomainDefPtr vmDef = args->vmDef;
> > > + char *path;
> > > +
> > > + if (virSetUIDGID(uid, gid, NULL, 0) < 0) {
> > > + virReportSystemError(errno, "%s",
> > > + _("setuid or setgid failed"));
> > > + _exit(-1);
> > > + }
> > > +
> > > + for (i = 0; i < vmDef->nfss; i++) {
> > > + path = vmDef->fss[i]->src;
> > > + if (access(path, R_OK) || access(path, W_OK) ||
> > > virFileIsExecutable(path)) {
> > > + VIR_DEBUG("Src dir '%s' does not belong to uid/gid: %d/%d",
> > > + vmDef->fss[i]->src, uid, gid);
> > > + _exit(-1);
> > > + }
> > > + }
> > > +
> > > + _exit(0);
> > > +}
> > > +
> > > +/*
> > > + * Helper function for helping check
> > > + * whether we have enough privilege
> > > + * to operate the source dir when userns enabled
> > > + * @vmDef: pointer to vm definition structure
> > > + * Returns 0 on success or -1 in case of error
> > > + */
> > > +static int
> > > +lxcContainerCheckDirPermission(virDomainDefPtr vmDef)
> > > +{
> > > + uid_t uid;
> > > + gid_t gid;
> > > + int cpid = 0;
> > > + int status;
> > > + char *childStack;
> > > + char *stack;
> > > + int flags = SIGCHLD;
> > > +
> > > + uid = vmDef->idmap.uidmap[0].target;
> > > + gid = vmDef->idmap.gidmap[0].target;
> > > +
> > > + lxc_userns_DirPermCheck_argv_t args = {
> > > + .uid = uid,
> > > + .gid = gid,
> > > + .vmDef = vmDef
> > > + };
> > > +
> > > + if (VIR_ALLOC_N(stack, getpagesize() * 4) < 0)
> > > + return -1;
> > > +
> > > + childStack = stack + (getpagesize() * 4);
> > > + cpid = clone(lxcContainerCheckDirPermissionChild, childStack,
> flags,
> > > &args);
> > > + VIR_FREE(stack);
> > > + if (cpid < 0) {
> > > + virReportSystemError(errno, "%s",
> > > + _("Unable to clone to check permission
> > of
> > > directory"));
> > > + return -1;
> > > + } else if (virProcessWait(cpid, &status) < 0) {
> > > + return -1;
> > > + }
> > > +
> > > + if (WEXITSTATUS(status) != 0) {
> > > + virReportSystemError(errno, "%s",
> > > + _("Check the permission of source dir
> > > provided for container"));
> > > + return -1;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > /**
> > > * lxcContainerStart:
> > > * @def: pointer to virtual machine structure
> > > @@ -1880,6 +1965,9 @@ int lxcContainerStart(virDomainDefPtr def,
> > > if (userns_supported()) {
> > > VIR_DEBUG("Enable user namespace");
> > > cflags |= CLONE_NEWUSER;
> > > + if (lxcContainerCheckDirPermission(def) < 0) {
> > > + return -1;
> > > + }
> > > } else {
> > > virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED,
> > > "%s",
> > > _("Kernel doesn't support user
> > > namespace"));
> > > --
> > > 1.8.2.1
This patch is basically about improving the error message from
LXC when permissions are inadequate. Over the past week I've
pushed a large number of patches to fix LXC error reporting
during startup. As such, I'm not sure that we still need to
have this patch - can you try latest GIT master and show what
error message you get with existing code.
I'd really rather avoid checking permissions ourselves, if we
have got error reporting at time of use working properly
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
More information about the libvir-list
mailing list