| Message ID | 1409725087-30483-1-git-send-email-cyliu@suse.com |
|---|---|
| State | New |
| Headers | show |
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
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. >
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.
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);
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(+)