"Daniel P. Berrange" <berrange@xxxxxxxxxx> writes: > On Tue, Sep 02, 2014 at 03:40:42PM +0800, Chunyan Liu wrote: >> To use virtio-serial device, unix socket created for chardev with >> default umask(022) has insufficient permissions. >> >> e.g. start kvm guest with: >> -device virtio-serial \ >> -chardev socket,path=/tmp/foo,server,nowait,id=foo \ >> -device virtserialport,chardev=foo,name=org.fedoraproject.port.0 >> >> Check permissions for the socket file that has been created in the host >> to enable communication through virtual serial ports in the guest: >> #ls -l /tmp/somefile.sock >> srwxr-xr-x 1 qemu qemu 0 21. Jul 14:19 /tmp/somefile.sock >> >> Other users in the qemu group (like real user, test engines, etc) cannot >> write to this socket. >> >> Problem reported here: >> https://sourceware.org/bugzilla/show_bug.cgi?id=13078#c11 >> https://bugzilla.novell.com/show_bug.cgi?id=888166 >> >> This patch tries to add a 'umask' option to 'chardev', so that user >> can have chance to indicate a umask overwritting the default one (default >> is 022), then create unix sockets with expected permissions. >> >> Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx> >> --- >> This is patch for qemu. >> >> qemu-char.c | 3 +++ >> qemu-options.hx | 9 +++++++-- >> util/qemu-sockets.c | 12 +++++++++++- >> 3 files changed, 21 insertions(+), 3 deletions(-) > >> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c >> index 5d38395..facf2c6 100644 >> --- a/util/qemu-sockets.c >> +++ b/util/qemu-sockets.c >> @@ -680,7 +680,8 @@ int unix_listen_opts(QemuOpts *opts, Error **errp) >> { >> struct sockaddr_un un; >> const char *path = qemu_opt_get(opts, "path"); >> - int sock, fd; >> + int newmask = qemu_opt_get_number(opts, "umask", 0); >> + int sock, fd, oldmask; >> >> sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0); >> if (sock < 0) { >> @@ -708,10 +709,19 @@ int unix_listen_opts(QemuOpts *opts, Error **errp) >> } >> >> unlink(un.sun_path); >> + if (newmask) { >> + oldmask = umask(newmask); >> + } >> if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) { >> + if (newmask) { >> + umask(oldmask); >> + } >> error_set_errno(errp, errno, QERR_SOCKET_BIND_FAILED); >> goto err; >> } >> + if (newmask) { >> + umask(oldmask); >> + } > > Setting umask() is not thread-safe as it affects the entire process. While > this is OK for chardevs which are cold-plugged at startup, once QEMU is > running it is not OK to alter umask during hotplug of devices. Indeed. No telling what else could be affected while the temporary umask is in effect. > Wouldn't it be simpler for libvirt to simply set the umask to 002 when it > first launches QEMU, avoiding the need for trying todo this per device. I guess that's okay as long as you also make sure QEMU's effective group is sane. If you must have laxer permissions just for a specific character device, you can perhaps chmod() after creating it. No good for tightening permissions, of course. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list