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

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

[Chunyan Liu]

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

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

[Daniel P. Berrange]

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 :|

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 496 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]