There are a few situations where passt itself is unable to create a file because it runs under QEMU user (e.g. just like our example from formatdomain.rst suggests: /var/log/passt.log). If libvirtd runs with sufficient permissions (e.g. as root) it can create the file and set seclabels on it so that passt can then open it. Ideally, we would just pass pre-opened FD, but this wasn't viewed as secure enough [1]. So lets just create the file and set seclabels. For the case when both libvirtd and passt have the same permissions, well then we fail before even needing to fork() and exec(). 1: https://archives.passt.top/passt-dev/20230606225836.63aecebe@elisabeth/ Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2209191 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_passt.c | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c index 99636a3a49..25b22d8ad9 100644 --- a/src/qemu/qemu_passt.c +++ b/src/qemu/qemu_passt.c @@ -20,6 +20,8 @@ #include <config.h> +#include <fcntl.h> + #include "qemu_dbus.h" #include "qemu_extdevice.h" #include "qemu_security.h" @@ -136,9 +138,13 @@ void qemuPasstStop(virDomainObj *vm, virDomainNetDef *net) { + qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net); g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net); + qemuSecurityDomainRestorePathLabel(driver, vm, net->backend.logFile); + qemuPasstKill(pidfile, passtSocketName); } @@ -166,10 +172,12 @@ qemuPasstStart(virDomainObj *vm, { qemuDomainObjPrivate *priv = vm->privateData; virQEMUDriver *driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net); g_autoptr(virCommand) cmd = NULL; g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net); char macaddr[VIR_MAC_STRING_BUFLEN]; + bool needUnlink = false; size_t i; cmd = virCommandNew(PASST); @@ -191,8 +199,25 @@ qemuPasstStart(virDomainObj *vm, if (net->sourceDev) virCommandAddArgList(cmd, "--interface", net->sourceDev, NULL); - if (net->backend.logFile) + if (net->backend.logFile) { + VIR_AUTOCLOSE logfd = -1; + /* The logFile location is not restricted to a per-domain directory. It + * can be anywhere. Pre-create it as passt may not have enough perms to + * do so. */ + if (qemuDomainOpenFile(cfg, vm->def, net->backend.logFile, + O_CREAT | O_TRUNC | O_APPEND | O_RDWR, + &needUnlink) < 0) { + return -1; + } + + if (qemuSecurityDomainSetPathLabel(driver, vm, + net->backend.logFile, false) < 0) { + goto error; + } + + /* Worse, passt deliberately doesn't support FD passing. */ virCommandAddArgList(cmd, "--log-file", net->backend.logFile, NULL); + } /* Add IP address info */ for (i = 0; i < net->guestIP.nips; i++) { @@ -203,7 +228,7 @@ qemuPasstStart(virDomainObj *vm, * a single IPv4 and single IPv6 address */ if (!(addr = virSocketAddrFormat(&ip->address))) - return -1; + goto error; virCommandAddArgList(cmd, "--address", addr, NULL); @@ -231,14 +256,14 @@ qemuPasstStart(virDomainObj *vm, /* validation guarantees this will never happen */ virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid portForward proto value %1$u"), pf->proto); - return -1; + goto error; } if (VIR_SOCKET_ADDR_VALID(&pf->address)) { g_autofree char *addr = NULL; if (!(addr = virSocketAddrFormat(&pf->address))) - return -1; + goto error; virBufferAddStr(&buf, addr); emitsep = true; @@ -284,7 +309,7 @@ qemuPasstStart(virDomainObj *vm, if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0) - return -1; + goto error; if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, true, NULL) < 0) goto error; @@ -292,6 +317,11 @@ qemuPasstStart(virDomainObj *vm, return 0; error: + if (needUnlink && unlink(net->backend.logFile) < 0) { + VIR_WARN("Unable to unlink '%s': %s", + net->backend.logFile, g_strerror(errno)); + } + qemuPasstKill(pidfile, passtSocketName); return -1; } -- 2.39.3