This turns out to be working by magic but needs to be fixed. --- src/security/virt-aa-helper.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e4d9a76..81f9f40 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1251,6 +1251,12 @@ main(int argc, char **argv) exit(EXIT_FAILURE); } + if (virThreadInitialize() < 0 || + virErrorInitialize() < 0) { + fprintf(stderr, _("%s: initialization failed\n"), argv[0]); + exit(EXIT_FAILURE); + } + /* clear the environment */ environ = NULL; if (setenv("PATH", "/sbin:/usr/sbin", 1) != 0) -- 2.1.2

To get virt-sandbox-service working with AppArmor, virt-aa-helper needs not to choke on path in /etc/libvirt-sandbox/services. --- src/security/virt-aa-helper.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 81f9f40..9c9e570 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -608,8 +608,12 @@ valid_path(const char *path, const bool readonly) npaths = sizeof(restricted)/sizeof(*(restricted)); if (array_starts_with(path, restricted, npaths) == 0 && - array_starts_with(path, override, opaths) != 0) - return 1; + array_starts_with(path, override, opaths) != 0) { + + /* We want to have virt-sandbox services config allowed */ + if (!STRPREFIX(path, "/etc/libvirt-sandbox/services/")) + return 1; + } npaths = sizeof(restricted_rw)/sizeof(*(restricted_rw)); if (!readonly) { -- 2.1.2

On Tue, 2014-11-25 at 08:05 +0100, Martin Kletzander wrote: Oh indeed! I overlooked that one. I'll change that. -- Cedric

On Mon, Nov 24, 2014 at 09:54:44PM +0100, Cédric Bosdonnat wrote: I agree, the 'name' was always there, just optional. But what version of iproute2 do you have that requires it? I checked the current HEAD and it's still optional. This must be a bug in that particular implementation. ACK if you can argue with the version or platform this is required on. Martin

On Tue, Nov 25, 2014 at 09:21:10AM +0100, Cedric Bosdonnat wrote: Must be OpenSUSE specific. Anyway, we need to work with that as well. ACK then ;) Martin

On Tue, Nov 25, 2014 at 04:19:48PM +0100, Richard Weinberger wrote: Oh, thank you for finding that, I should've done my homework! Since it really is just a bug on iproute2 side in openSUSE, I'd rather keep it in its original state. And since the patch is already pushed, I'm inclining to reverting it. Other opinions? Martin

Resolving symlinks can fail before mounting any file system if one file system depends on another being mounted. Symlinks are now resolved in two passes: * Before any file system is mounted, but then we are more gentle if the source path can't be accessed * Right before mounting a file system, so that we are sure that we have the resolved path... but then if it can't be accessed we raise an error. --- src/conf/domain_conf.h | 1 + src/lxc/lxc_container.c | 77 ++++++++++++++++++++++++++++++++++--------------- 2 files changed, 54 insertions(+), 24 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0a609df..f56b8b1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -821,6 +821,7 @@ struct _virDomainFSDef { virDomainDeviceInfo info; unsigned long long space_hard_limit; /* in bytes */ unsigned long long space_soft_limit; /* in bytes */ + bool symlinksResolved; }; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index db823d6..12f3a41 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -608,6 +608,48 @@ static int lxcContainerUnmountSubtree(const char *prefix, return ret; } +static int lxcContainerResolveSymlinks(virDomainFSDefPtr fs, bool gentle) +{ + char *newroot; + + if (!fs->src || fs->symlinksResolved) + return 0; + + if (access(fs->src, F_OK)) { + if (gentle) { + /* Just ignore the error for the while, we'll try again later */ + VIR_DEBUG("Skipped unaccessible '%s'", fs->src); + return 0; + } else { + virReportSystemError(errno, + _("Failed to access '%s'"), fs->src); + return -1; + } + } + + VIR_DEBUG("Resolving '%s'", fs->src); + if (virFileResolveAllLinks(fs->src, &newroot) < 0) { + if (gentle) { + VIR_DEBUG("Skipped non-resolvable '%s'", fs->src); + return 0; + } else { + virReportSystemError(errno, + _("Failed to resolve symlink at %s"), + fs->src); + } + return -1; + } + + /* Mark it resolved to skip it the next time */ + fs->symlinksResolved = true; + + VIR_DEBUG("Resolved '%s' to %s", fs->src, newroot); + + VIR_FREE(fs->src); + fs->src = newroot; + + return 0; +} static int lxcContainerPrepareRoot(virDomainDefPtr def, virDomainFSDefPtr root, @@ -634,6 +676,9 @@ static int lxcContainerPrepareRoot(virDomainDefPtr def, return -1; } + if (lxcContainerResolveSymlinks(root, false) < 0) + return -1; + if (virAsprintf(&dst, "%s/%s.root", LXC_STATE_DIR, def->name) < 0) return -1; @@ -1552,6 +1597,9 @@ static int lxcContainerMountAllFS(virDomainDefPtr vmDef, if (STREQ(vmDef->fss[i]->dst, "/")) continue; + if (lxcContainerResolveSymlinks(vmDef->fss[i], false) < 0) + return -1; + if (lxcContainerUnmountSubtree(vmDef->fss[i]->dst, false) < 0) return -1; @@ -1735,37 +1783,18 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, return ret; } - -static int lxcContainerResolveSymlinks(virDomainDefPtr vmDef) +static int lxcContainerResolveAllSymlinks(virDomainDefPtr vmDef) { - char *newroot; size_t i; VIR_DEBUG("Resolving symlinks"); for (i = 0; i < vmDef->nfss; i++) { virDomainFSDefPtr fs = vmDef->fss[i]; - if (!fs->src) - continue; - - if (access(fs->src, F_OK)) { - virReportSystemError(errno, - _("Failed to access '%s'"), fs->src); + /* In the first pass, be gentle as some files may + depend on other filesystems to be mounted */ + if (lxcContainerResolveSymlinks(fs, true) < 0) return -1; - } - - VIR_DEBUG("Resolving '%s'", fs->src); - if (virFileResolveAllLinks(fs->src, &newroot) < 0) { - virReportSystemError(errno, - _("Failed to resolve symlink at %s"), - fs->src); - return -1; - } - - VIR_DEBUG("Resolved '%s' to %s", fs->src, newroot); - - VIR_FREE(fs->src); - fs->src = newroot; } VIR_DEBUG("Resolved all filesystem symlinks"); @@ -2106,7 +2135,7 @@ static int lxcContainerChild(void *data) goto cleanup; } - if (lxcContainerResolveSymlinks(vmDef) < 0) + if (lxcContainerResolveAllSymlinks(vmDef) < 0) goto cleanup; VIR_DEBUG("Setting up pivot"); -- 2.1.2

The typical case where we had a problem is with such a filesystem definition as created by virt-sandbox-service: <filesystem type='bind' accessmode='passthrough'> <source dir='/var/lib/libvirt/filesystems/mysshd/var'/> <target dir='/var'/> </filesystem> In this case, we don't want to unmount the /var subtree or we may loose the access to the source folder. --- src/lxc/lxc_container.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 12f3a41..334a1df 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1597,11 +1597,15 @@ static int lxcContainerMountAllFS(virDomainDefPtr vmDef, if (STREQ(vmDef->fss[i]->dst, "/")) continue; + VIR_DEBUG("Mounting '%s' -> '%s'", vmDef->fss[i]->src, vmDef->fss[i]->dst); + if (lxcContainerResolveSymlinks(vmDef->fss[i], false) < 0) return -1; - if (lxcContainerUnmountSubtree(vmDef->fss[i]->dst, - false) < 0) + + if (!(vmDef->fss[i]->src && + STRPREFIX(vmDef->fss[i]->src, vmDef->fss[i]->dst)) && + lxcContainerUnmountSubtree(vmDef->fss[i]->dst, false) < 0) return -1; if (lxcContainerMountFS(vmDef->fss[i], sec_mount_options) < 0) -- 2.1.2

On Tue, 2014-11-25 at 08:48 +0100, Martin Kletzander wrote: Indeed that happens when the host root is mounted as the container root... and that's what virt-sandbox-service does. We have this situation when the libvirt-sandbox service has a disk image: * The disk image is mounted to /var/lib/libvirt/filesystems/<name> * Quite a few items from /var/lib/libvirt/filesystems/<name> are bind mounted to their equivalent in the container root, and /var is one of them. -- Cedric