On Tue, Apr 18, 2023 at 02:59:26 -0400, Nick Guenther wrote: > The qemu driver creates IPC sockets using absolute paths, > but under POSIX socket paths are constrained pretty tightly. > On systems with homedirs on an unusual mount point, like > network homedirs, or just particularly long usernames, this > could make starting VMs under qemu:///session impossible. > > Resolves https://gitlab.com/libvirt/libvirt/-/issues/466 > > Signed-off-by: Nick Guenther <nick.guenther@xxxxxxxxxx> > --- > src/qemu/qemu_command.c | 52 ++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 49 insertions(+), 3 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 4ca93bf3dc..3f180d5fb6 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -53,6 +53,7 @@ > #include "virmdev.h" > #include "virutil.h" > > +#include <libgen.h> > #include <sys/stat.h> > #include <fcntl.h> > > @@ -4866,6 +4867,37 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev) > struct sockaddr_un addr; > socklen_t addrlen = sizeof(addr); > int fd; > + char* wd = NULL; > + char* socket_dir_c = NULL; > + char* socket_name_c = NULL; > + char* socket_dir = NULL; > + char* socket_name = NULL; Apart from various cases of code style not being aligned from what libvirt does normally ... > + > + /* The path length is limited to what fits in sockaddr_un. > + * It's pretty short: 108 on Linux, and this is too easy to hit. > + * Work around this limit by using a *relative path*. > + * > + * background: https://stackoverflow.com/questions/34829600/why-is-the-maximal-path-length-allowed-for-unix-sockets-on-linux-108 > + * > + * docker added a different workaround: https://github.com/moby/moby/pull/13408 > + */ > + if ((wd = getcwd(NULL, 0)) == NULL) { > + virReportSystemError(errno, "%s", > + _("Unable to get working directory")); > + goto error; > + } > + > + socket_dir_c = strdup(dev->data.nix.path); // dirname edits the string given it, so it must be copied > + socket_name_c = strdup(dev->data.nix.path); > + > + socket_dir = dirname(socket_dir_c); > + socket_name = basename(socket_name_c); > + > + if (chdir(socket_dir) < 0) { This will not work in a multi-threaded process, which can potentially call this function multiple times at the same time for starting multiple VMs. One of the threads changes the directory of the process, second thread changes it again and then one of the threads creates the socket in the wrong directory. > + virReportSystemError(errno, "%s", > + _("Unable to get change to socket directory")); > + goto error; > + } > > if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { > virReportSystemError(errno, "%s", > @@ -4875,10 +4907,10 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev) > > memset(&addr, 0, sizeof(addr)); > addr.sun_family = AF_UNIX; > - if (virStrcpyStatic(addr.sun_path, dev->data.nix.path) < 0) { > + if (virStrcpyStatic(addr.sun_path, socket_name) < 0) { > virReportError(VIR_ERR_INTERNAL_ERROR, > - _("UNIX socket path '%1$s' too long"), > - dev->data.nix.path); > + _("UNIX socket name '%1$s' too long"), > + socket_name); > goto error; Also any failure here doesn't restore the directory. > } > > @@ -4909,9 +4941,23 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef *dev) > if (virFileUpdatePerm(dev->data.nix.path, 0002, 0664) < 0) > goto error; > > + /* restore working directory */ > + if (chdir(wd) < 0) { > + virReportSystemError(errno, "%s", > + _("Unable to restore working directory")); > + goto error; > + } > + > + free(socket_name_c); > + free(socket_dir_c); > + free(wd); > + > return fd; > > error: > + if (socket_name_c != NULL) { free(socket_name_c); } > + if (socket_dir_c != NULL) { free(socket_dir_c); } > + if (wd != NULL) { free(wd); } > VIR_FORCE_CLOSE(fd); > return -1; > } > -- > 2.34.1 >
