Hi Aaron,
I'm still a little bit nervous about calling chown on a (partially) user
controlled file name.
Before moving forward I wanted to discuss a couple of other options:
* Ansis (in CC) suggested using -runas parameter in qemu. This way qemu can
open the socket as root and drop privileges before starting guest execution.
* Instead of changing the owner of the socket, we could create it as a
different user using setfsgid() or seteuid() system call. I'm not sure whether
this is doable, we need to make sure that rte_vhost_driver_register() can run
with reduced privileges and that the system calls play nicely with other
threads.
What do you guys think?
Thanks,
Daniele
On 20/05/2016 13:32, "Aaron Conole" <acon...@redhat.com> wrote:
>Currently, when dpdkvhostuser devices are created, they inherit whatever the
>running umask and uid/gid of the vswitchd process. This leads to difficulties
>when using vhost_user consumers (such as qemu).
>
>This patch introduces two new database entries, 'vhost-sock-owner' to set the
>ownership, and 'vhost-sock-perms' to set the permissions bits for the
>vhost_user sockets.
>
>Signed-off-by: Aaron Conole <acon...@redhat.com>
>---
>v1->v2:
>* Rebased to latest
>
> INSTALL.DPDK.md | 7 +++++++
> NEWS | 2 ++
> lib/netdev-dpdk.c | 26 +++++++++++++++++++++++---
> vswitchd/vswitch.xml | 23 +++++++++++++++++++++++
> 4 files changed, 55 insertions(+), 3 deletions(-)
>
>diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
>index 93f92e4..5debcd1 100644
>--- a/INSTALL.DPDK.md
>+++ b/INSTALL.DPDK.md
>@@ -180,6 +180,13 @@ Using the DPDK with ovs-vswitchd:
> * vhost-sock-dir
> Option to set the path to the vhost_user unix socket files.
>
>+ * vhost-sock-owner
>+ Option to set the owner of the vhost_user unix socket files.
>+
>+ * vhost-sock-perms
>+ Option to set the DAC permissions of the vhost_user unix socket
>+ files.
>+
> NOTE: Changing any of these options requires restarting the ovs-vswitchd
> application.
>
>diff --git a/NEWS b/NEWS
>index 4e81cad..807d4bf 100644
>--- a/NEWS
>+++ b/NEWS
>@@ -32,6 +32,8 @@ Post-v2.5.0
> * DB entries have been added for many of the DPDK EAL command line
> arguments. Additional arguments can be passed via the dpdk-extra
> entry.
>+ * New database options 'vhost-sock-owner' and 'vhost-sock-perms' for
>+ setting vhost_user unix socket file ownership and permissions.
> - ovs-benchmark: This utility has been removed due to lack of use and
> bitrot.
> - ovs-appctl:
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index 87879d5..d405e66 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -31,6 +31,7 @@
> #include <sys/stat.h>
> #include <getopt.h>
>
>+#include "chutil.h"
> #include "dirs.h"
> #include "dp-packet.h"
> #include "dpif-netdev.h"
>@@ -140,6 +141,10 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF /
>ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF))
> static char *cuse_dev_name = NULL; /* Character device cuse_dev_name. */
> #endif
> static char *vhost_sock_dir = NULL; /* Location of vhost-user sockets */
>+static char *vhost_sock_def_owner = NULL; /* Default owner of vhost-user
>+ sockets*/
>+static char *vhost_sock_def_perms = NULL; /* Default permissions of
>+ vhost-user sockets */
>
> /*
> * Maximum amount of time in micro seconds to try and enqueue to vhost.
>@@ -872,6 +877,16 @@ netdev_dpdk_vhost_user_construct(struct netdev *netdev)
> }
>
> ovs_mutex_unlock(&dpdk_mutex);
>+ if (!err && vhost_sock_def_owner &&
>+ (err = ovs_chown(dev->vhost_id, vhost_sock_def_owner))) {
>+ VLOG_ERR("vhost-user socket device ownership change failed.");
>+ }
>+
>+ if (!err && vhost_sock_def_perms &&
>+ (err = ovs_chmod(dev->vhost_id, vhost_sock_def_perms))) {
>+ VLOG_ERR("vhost-user socket device permission change failed.");
>+ }
>+
> return err;
> }
>
>@@ -3098,8 +3113,8 @@ dpdk_init__(const struct smap *ovs_other_config)
> VLOG_INFO("DPDK Enabled, initializing");
>
> #ifdef VHOST_CUSE
>- if (process_vhost_flags("cuse-dev-name", xstrdup("vhost-net"),
>- PATH_MAX, ovs_other_config, &cuse_dev_name)) {
>+ process_vhost_flags("cuse-dev-name", xstrdup("vhost-net"),
>+ PATH_MAX, ovs_other_config, &cuse_dev_name);
> #else
> if (process_vhost_flags("vhost-sock-dir", xstrdup(ovs_rundir()),
> NAME_MAX, ovs_other_config,
>@@ -3123,9 +3138,14 @@ dpdk_init__(const struct smap *ovs_other_config)
> free(sock_dir_subcomponent);
> } else {
> vhost_sock_dir = sock_dir_subcomponent;
>-#endif
> }
>
>+ process_vhost_flags("vhost-sock-owner", NULL, NAME_MAX, ovs_other_config,
>+ &vhost_sock_def_owner);
>+ process_vhost_flags("vhost-sock-perms", NULL, NAME_MAX, ovs_other_config,
>+ &vhost_sock_def_perms);
>+#endif
>+
> /* Get the main thread affinity */
> CPU_ZERO(&cpuset);
> err = pthread_getaffinity_np(pthread_self(), sizeof(cpu_set_t),
>diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>index 944d8ec..0553d5a 100644
>--- a/vswitchd/vswitch.xml
>+++ b/vswitchd/vswitch.xml
>@@ -311,6 +311,29 @@
> </p>
> </column>
>
>+ <column name="other_config" key="vhost-sock-owner"
>+ type='{"type": "string"}'>
>+ <p>
>+ Specifies the owner of the vhost-user unix domain socket files.
>+ </p>
>+ <p>
>+ The default is to inherit from the running user and group id's. The
>+ argument is specified in the same form as the 'chown' unix utility.
>+ </p>
>+ </column>
>+
>+ <column name="other_config" key="vhost-sock-perms"
>+ type='{"type": "string"}'>
>+ <p>
>+ Specifies the permissions for the vhost-user unix domain socket
>+ files.
>+ </p>
>+ <p>
>+ The default is derived from the running mask. The argument is
>+ specified in the same form as the 'chmod' unix utility.
>+ </p>
>+ </column>
>+
> <column name="other_config" key="n-handler-threads"
> type='{"type": "integer", "minInteger": 1}'>
> <p>
>--
>2.5.5
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
