[ovs-dev] [PATCH v4 0/3] vhost-user: Add the ability to control ownership/permissions
aconole at redhat.com
Tue Oct 4 12:21:57 UTC 2016
Daniele Di Proietto <diproiettod at ovn.org> writes:
> Hi Aaron, apologies for the delay,
> I took another look at the series, and I'm not convinced this is the right solution to the problem.
> This obviously has nothing to do with the quality of the code.
> While the hardness amplification might help against symlinks, it doesn't protect against some
> type of race conditions.
> There's still a window between 'rte_vhost_driver_register()' and 'vhost_set_permissions()' that
> can be exploited to change permissions of other sockets in the ovs rundir (which is the default
> vhostuser socket directory). ovs_kchown() doesn't guarantee that the socket is the one we
> intended. By introducing a sleep between 'rte_vhost_driver_register()' and
> 'vhost_set_permissions()' (and restarting some daemons in between) I was able to trick
> ovs-vswitchd into changing the permissions of the ovn nb database socket. I suspect someone
> more expert than me could find similar and more dangerous exploits.
> Alternative approaches:
> 1) vhostuser client mode. It prevents this (and other) problem(s) and it is IMHO the best way to
> deploy vhostuser in production. I know that it requires a recent version of QEMU, but this whole
> stack is still pretty new, I don't see many ways around this.
> 2) Do we want to do chmod/chown in the lower layers of the software stack? Then we need
> some support in DPDK. Using fchmod() and fchown() in DPDK seems better to me, because it's
> possible to do it without any race conditions. The same thing is not possible in OVS (it would be
> possible if DPDK gave us access to the fd before binding).
> 3) If it's not possible to do it in the lower layers without any races, I think it's best not to hide the
> hack, but to do it in outside of Open vSwitch, where there might be more context about the
> other running daemons, or the vhost-sock-dir. As mentioned before, a manual intervention will
> not survive an ovs-vswitchd restart, but an ovs restart will also break the connection to the
> existing VM (that's why I think client mode is a much better approach).
Agreed. Let's drop this effort.
> Again, this is just my opinion, but I'm really not comfortable maintaining this.
> 2016-08-19 16:48 GMT-07:00 Aaron Conole <aconole at redhat.com>:
> Currently, when using Open vSwitch with DPDK and qemu guests, the recommended
> method for joining the guests is via the dpdkvhostuser interface. This
> interface uses Unix Domain sockets to communicate. When these sockets are
> created, they inherit the permissions and ownership from the vswitchd process.
> This can lead to an undesirable state where the QEMU process cannot use the
> socket file until manual intervention is performed (via `chown` and/or `chmod`
> This patchset gives the ability to set the permissions and ownership of all
> dpdkvhostuser sockets from the database, avoiding the manual intervention
> required to connect QEMU and OVS via DPDK.
> The first patch adds chmod and chown calls to lib, with unit tests. The
> second patch adds a hardness amplification version as described in the
> paper "Portably Solving File TOCTTOU Races with Hardness Amplification"
> found at
> while the third patch hooks those calls into the
> netdev_dpdk_vhost_user_construct function, after the socket is created.
> Changes from v3:
> * Replaced patch 2/3 with hardness amplification version. Retested on RHEL7
> and validated the travis builds.
> Changes from v2:
> * Added a new 2nd patch to series for chmod/chown on already opened files.
> There exist known implementations for other systems, including FreeBSD, but
> only linux is implemented. ENOTSUP is set when these calls fail on non-linux
> Aaron Conole (3):
> chutil: introduce a new change-utils lib
> chutil: Add hardness amplification versions of chmod/chown
> netdev-dpdk: Support user-defined socket attribs
> INSTALL.DPDK.md | 8 +
> configure.ac | 2 +-
> lib/automake.mk | 2 +
> lib/chutil-unix.c | 652
> lib/chutil.h | 36 +++
> lib/daemon-unix.c | 149 +-----------
> lib/netdev-dpdk.c | 37 +++
> tests/automake.mk | 2 +
> tests/library.at | 5 +
> tests/test-chutil.c | 297 +++++++++++++++++++++++
> vswitchd/vswitch.xml | 23 ++
> 11 files changed, 1068 insertions(+), 145 deletions(-)
> create mode 100644 lib/chutil-unix.c
> create mode 100644 lib/chutil.h
> create mode 100644 tests/test-chutil.c
> dev mailing list
> dev at openvswitch.org
More information about the dev