[libvirt] [PATCH V2] fix: unix sockets created for virtio-serail has insufficient permissions

Add umask to _virCommand, allow user to set umask to command. Set umask(002) to qemu process to overwrite default umask(022) so that unix sockets created for virtio-serial has expected permissions. Fix problem reported here: https://sourceware.org/bugzilla/show_bug.cgi?id=13078#c11 https://bugzilla.novell.com/show_bug.cgi?id=888166 To use virtio-serial device, unix socket created for chardev with default umask(022) has insufficient permissions. e.g.: -device virtio-serial \ -chardev socket,path=/tmp/foo,server,nowait,id=foo \ -device virtserialport,chardev=foo,name=org.fedoraproject.port.0 srwxr-xr-x 1 qemu qemu 0 21. Jul 14:19 /tmp/somefile.sock Other users in the same group (like real user, test engines, etc) cannot write to this socket. Signed-off-by: Chunyan Liu <cyliu@suse.com> --- Changes: * set umask(002) to the whole qemu process instead of calling umask in qemu unix_listen_opts. V1 is here: http://www.mail-archive.com/libvir-list@redhat.com/msg101513.html src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 1 + src/util/vircommand.c | 11 +++++++++++ src/util/vircommand.h | 1 + 4 files changed, 14 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 71fc063..f136d24 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1171,6 +1171,7 @@ virCommandSetPidFile; virCommandSetPreExecHook; virCommandSetSELinuxLabel; virCommandSetUID; +virCommandSetUmask; virCommandSetWorkingDirectory; virCommandToString; virCommandWait; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f68dfbe..f76aa5a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4141,6 +4141,7 @@ int qemuProcessStart(virConnectPtr conn, virCommandSetPreExecHook(cmd, qemuProcessHook, &hookData); virCommandSetMaxProcesses(cmd, cfg->maxProcesses); virCommandSetMaxFiles(cmd, cfg->maxFiles); + virCommandSetUmask(cmd, 0x002); VIR_DEBUG("Setting up security labelling"); if (virSecurityManagerSetChildProcessLabel(driver->securityManager, diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 1d6dbd9..efb5f69 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -133,6 +133,7 @@ struct _virCommand { #if defined(WITH_SECDRIVER_APPARMOR) char *appArmorProfile; #endif + int umask; }; /* See virCommandSetDryRun for description for this variable */ @@ -582,6 +583,8 @@ virExec(virCommandPtr cmd) /* child */ + if (cmd->umask) + umask(cmd->umask); ret = EXIT_CANCELED; openmax = sysconf(_SC_OPEN_MAX); if (openmax < 0) { @@ -1082,6 +1085,14 @@ virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files) cmd->maxFiles = files; } +void virCommandSetUmask(virCommandPtr cmd, int umask) +{ + if (!cmd || cmd->has_error) + return; + + cmd->umask = umask; +} + /** * virCommandClearCaps: * @cmd: the command to modify diff --git a/src/util/vircommand.h b/src/util/vircommand.h index d3b286d..bf65de4 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -72,6 +72,7 @@ void virCommandSetUID(virCommandPtr cmd, uid_t uid); void virCommandSetMaxMemLock(virCommandPtr cmd, unsigned long long bytes); void virCommandSetMaxProcesses(virCommandPtr cmd, unsigned int procs); void virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files); +void virCommandSetUmask(virCommandPtr cmd, int umask); void virCommandClearCaps(virCommandPtr cmd); -- 1.8.4.5

On Wed, Sep 03, 2014 at 02:18:07PM +0800, Chunyan Liu wrote:
Add umask to _virCommand, allow user to set umask to command. Set umask(002) to qemu process to overwrite default umask(022) so that unix sockets created for virtio-serial has expected permissions.
Fix problem reported here: https://sourceware.org/bugzilla/show_bug.cgi?id=13078#c11 https://bugzilla.novell.com/show_bug.cgi?id=888166
To use virtio-serial device, unix socket created for chardev with default umask(022) has insufficient permissions. e.g.: -device virtio-serial \ -chardev socket,path=/tmp/foo,server,nowait,id=foo \ -device virtserialport,chardev=foo,name=org.fedoraproject.port.0
srwxr-xr-x 1 qemu qemu 0 21. Jul 14:19 /tmp/somefile.sock
Other users in the same group (like real user, test engines, etc) cannot write to this socket.
Signed-off-by: Chunyan Liu <cyliu@suse.com> --- Changes: * set umask(002) to the whole qemu process instead of calling umask in qemu unix_listen_opts.
V1 is here: http://www.mail-archive.com/libvir-list@redhat.com/msg101513.html
src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 1 + src/util/vircommand.c | 11 +++++++++++ src/util/vircommand.h | 1 + 4 files changed, 14 insertions(+)
ACK, you could possibly argue that it should be configurable in qemu.conf, but I think that would be overkill. We unconditionally put QEMU into a special group, so we really should make sure that stuff is accessible to that group via umask. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 09/03/2014 02:49 AM, Daniel P. Berrange wrote:
On Wed, Sep 03, 2014 at 02:18:07PM +0800, Chunyan Liu wrote:
Add umask to _virCommand, allow user to set umask to command. Set umask(002) to qemu process to overwrite default umask(022) so that unix sockets created for virtio-serial has expected permissions.
ACK, you could possibly argue that it should be configurable in qemu.conf, but I think that would be overkill. We unconditionally put QEMU into a special group, so we really should make sure that stuff is accessible to that group via umask.
Pushed with amended subject line. It would also be nice to enhance tests/commandtest.c to cover this addition, but that can be a separate patch. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 09/03/2014 12:18 AM, Chunyan Liu wrote: s/serail/serial/ in the subject Actually, I'd go with a broader subject line: qemu: ensure sane umask for qemu process
Add umask to _virCommand, allow user to set umask to command. Set umask(002) to qemu process to overwrite default umask(022) so that unix sockets created for virtio-serial has expected permissions.
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Chunyan Liu
-
Daniel P. Berrange
-
Eric Blake