[ovs-dev] [PATCH v11 4/8] netdev-dpdk: Restrict vhost_sock_dir

Aaron Conole aconole at redhat.com
Fri Apr 1 15:31:33 UTC 2016


Since the vhost-user sockets directory now comes from the database, it is
possible for any user with database access to program an arbitrary filesystem
location for the sockets directory. This could result in unprivileged users
creating or deleting arbitrary filesystem files by using specially crafted
names. To prevent this, use the introduced ovs_realpath function to resolve
the filesystem location and ensure that it resolves under the ovs_rundir.

Signed-off-by: Aaron Conole <aconole at redhat.com>
---
v11 (correct):
* Added

 INSTALL.DPDK.md      |  3 ++-
 lib/netdev-dpdk.c    | 30 ++++++++++++++++++++++--------
 vswitchd/vswitch.xml |  3 ++-
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 8397688..73fc876 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -182,7 +182,8 @@ Using the DPDK with ovs-vswitchd:
    Option to set the vhost_cuse character device name.
 
    * vhost-sock-dir
-   Option to set the path to the vhost_user unix socket files.
+   Option to set the path to the vhost_user unix socket files. This path must
+   exist and must be a sub-tree of the Open vSwitch running directory.
 
    NOTE: Changing any of these options requires restarting the ovs-vswitchd
    application.
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 152b66f..397a3fa 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -812,6 +812,12 @@ netdev_dpdk_vhost_user_construct(struct netdev *netdev_)
     const char *name = netdev_->name;
     int err;
 
+    if (!vhost_sock_dir) {
+        VLOG_ERR("Unable to setup a valid path to vhost-user sockets; "
+                 "Check the 'vhost-sock-dir' option.");
+        return EINVAL;
+    }
+
     /* 'name' is appended to 'vhost_sock_dir' and used to create a socket in
      * the file system. '/' or '\' would traverse directories, so they're not
      * acceptable in 'name'. */
@@ -2896,6 +2902,9 @@ dpdk_init__(const struct ovsrec_open_vswitch *ovs_cfg)
     int argc;
     int err;
     cpu_set_t cpuset;
+#ifndef VHOST_CUSE
+    char *sock_dir_subcomponent;
+#endif
 
     if (!smap_get_bool(&ovs_cfg->other_config, "dpdk-init", false)) {
         VLOG_INFO("DPDK Disabled - to change this requires a restart.\n");
@@ -2908,15 +2917,20 @@ dpdk_init__(const struct ovsrec_open_vswitch *ovs_cfg)
     if (process_vhost_flags("cuse-dev-name", xstrdup("vhost-net"),
                             PATH_MAX, ovs_cfg, &cuse_dev_name)) {
 #else
-    if (process_vhost_flags("vhost-sock-dir", xstrdup(ovs_rundir()),
-                            NAME_MAX, ovs_cfg, &vhost_sock_dir)) {
-        struct stat s;
-
-        err = stat(vhost_sock_dir, &s);
-        if (err) {
-            VLOG_ERR("vhost-user sock directory '%s' does not exist.",
-                     vhost_sock_dir);
+    if (process_vhost_flags("vhost-sock-dir", xstrdup(""),
+                            NAME_MAX, ovs_cfg, &sock_dir_subcomponent)) {
+        char *full_sockdir = xasprintf("%s/%s", ovs_rundir(),
+                                       sock_dir_subcomponent);
+        vhost_sock_dir = ovs_realpath(full_sockdir);
+        if (!vhost_sock_dir
+            || strncmp(vhost_sock_dir, ovs_rundir(), strlen(ovs_rundir()))) {
+            VLOG_ERR("Invalid vhost-user socket directory '%s/%s'.",
+                     ovs_rundir(), sock_dir_subcomponent);
+            free(vhost_sock_dir);
+            vhost_sock_dir = 0;
         }
+        free(full_sockdir);
+        free(sock_dir_subcomponent);
 #endif
     }
 
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index c350247..2a5f8fa 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -311,7 +311,8 @@
       <column name="other_config" key="vhost-sock-dir"
               type='{"type": "string"}'>
         <p>
-          Specifies the path to the vhost-user unix domain socket files.
+          Specifies the path to the vhost-user unix domain socket files. This
+          path must exist and be located within the working directory subtree.
         </p>
         <p>
           Defaults to the working directory of the application. Changing this
-- 
2.5.5




More information about the dev mailing list