Daniele Di Proietto <diproiet...@vmware.com> writes: > If vhost-sock-dir is empty, vhost_sock_dir will be NULL, because > process_vhost_flags() will return 0.
D'oh! Missed this in testing. I'll fix it, and make sure I test this case more thoroughly. Thanks so much for the review, Daniele! -Aaron > On 26/04/2016 12:42, "Aaron Conole" <acon...@redhat.com> wrote: > >>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 <acon...@redhat.com> >>--- >>Previous: http://openvswitch.org/pipermail/dev/2016-April/069030.html >> >>v12: >>* Converted to using strstr instead of canonicalization >> >> lib/netdev-dpdk.c | 27 ++++++++++++++++++++------- >> vswitchd/vswitch.xml | 4 +++- >> 2 files changed, 23 insertions(+), 8 deletions(-) >> >>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >>index a393cee..9793fc5 100644 >>--- a/lib/netdev-dpdk.c >>+++ b/lib/netdev-dpdk.c >>@@ -2897,6 +2897,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"); >>@@ -2909,15 +2912,25 @@ 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)) { >>+ if (process_vhost_flags("vhost-sock-dir", xstrdup(""), >>+ NAME_MAX, ovs_cfg, &sock_dir_subcomponent)) { >> 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 (!strstr(sock_dir_subcomponent, "..")) { >>+ vhost_sock_dir = xasprintf("%s/%s", ovs_rundir(), >>+ sock_dir_subcomponent); >>+ >>+ err = stat(vhost_sock_dir, &s); >>+ if (err) { >>+ VLOG_ERR("vhost-user sock directory '%s' does not exist.", >>+ vhost_sock_dir); >>+ } >>+ } else { >>+ vhost_sock_dir = xstrdup(ovs_rundir()); >>+ VLOG_ERR("vhost-user sock directory request '%s/%s' has invalid" >>+ "characters '..' - using %s instead.", >>+ ovs_rundir(), sock_dir_subcomponent, ovs_rundir()); >> } >>+ free(sock_dir_subcomponent); >> #endif >> } >> >>diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml >>index 5965b10..e6d7359 100644 >>--- a/vswitchd/vswitch.xml >>+++ b/vswitchd/vswitch.xml >>@@ -302,7 +302,9 @@ >> <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 a subdirectory tree of the Open vSwitch >>+ run directory. >> </p> >> <p> >> Defaults to the working directory of the application. Changing this >>-- >>2.5.5 >> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
