[adding virtiofs list] On Mon, Feb 03, 2020 at 04:43:51PM +0000, Daniel P. Berrangé wrote:
On Thu, Jan 30, 2020 at 06:06:26PM +0100, Ján Tomko wrote:Start virtiofsd for each <filesystem> device using it. Pre-create the socket for communication with QEMU and pass it to virtiofsd. Note that virtiofsd needs to run as root.So we're not able to use virtiofsd with the session libvirtd which runs completely unprivileged ?
Not with the version of virtiofsd currently merged in the QEMU tree.
I can understand the need to run as root if we want to support chown() of files, or DAC_OVERRIDE, but I'm surprised it isn't possible to run virtiofsd unprivileged & simply have a reduced featureset where it can only create files as that one user.
Apart from the possibly missing features (I don't know how well virtiofsd internals are ready for those), current version of the daemon sets up namespaces and the seccomp sandbox.
+int +qemuVirtioFSStart(virLogManagerPtr logManager, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainFSDefPtr fs) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virCommand) cmd = NULL; + g_autofree char *socket_path = NULL; + g_autofree char *pidfile = NULL; + g_autofree char *logpath = NULL; + pid_t pid = (pid_t) -1; + VIR_AUTOCLOSE fd = -1; + VIR_AUTOCLOSE logfd = -1; + int ret = -1; + int rc;+ + if (!(pidfile = qemuVirtioFSCreatePidFilename(cfg, vm->def, fs->info.alias))) + goto cleanup; + + if (!(socket_path = qemuVirtioFSCreateSocketFilename(vm, fs->info.alias))) + goto cleanup; + + if ((fd = qemuVirtioFSOpenChardev(driver, vm, socket_path)) < 0) + goto cleanup; + + logpath = qemuVirtioFSCreateLogFilename(cfg, vm->def, fs->info.alias); + + if (cfg->stdioLogD) { + if ((logfd = virLogManagerDomainOpenLogFile(logManager, + "qemu", + vm->def->uuid, + vm->def->name, + logpath, + 0, + NULL, NULL)) < 0) + goto cleanup; + } else { + if ((logfd = open(logpath, O_WRONLY | O_CREAT | O_APPEND, S_IRUSR | S_IWUSR)) < 0) { + virReportSystemError(errno, _("failed to create logfile %s"), + logpath); + goto cleanup; + } + if (virSetCloseExec(logfd) < 0) { + virReportSystemError(errno, _("failed to set close-on-exec flag on %s"), + logpath); + goto error; + } + } + + if (!(cmd = qemuVirtioFSBuildCommandLine(cfg, fs, &fd))) + goto cleanup; + + virCommandSetPidFile(cmd, pidfile); + virCommandSetOutputFD(cmd, &logfd); + virCommandSetErrorFD(cmd, &logfd); + virCommandNonblockingFDs(cmd); + virCommandDaemonize(cmd);We're not mandating "root" here, it is just inheriting the user that libvirtd runs as. So IIUC ,this will run virtofsd as non-root when used with session libvirtd, unless there's a check somewhere else that prevents this scenario ?
I'll add a check.
I'm also wondering about cgroups placement in this method, and any use of SELinux
Placing it into a cgroup should be easy, AFAIK it does not need to access any devices. As for SELinux, I don't think there's anything to be done other than updating the selinux-policy. Recursively relabeling the whole directory feels intrusive. Jano
+ + if (qemuExtDeviceLogCommand(driver, vm, cmd, "virtiofsd") < 0) + goto cleanup; + + rc = virCommandRun(cmd, NULL); + logfd = -1;
Attachment:
signature.asc
Description: PGP signature
