[ovs-dev] [PATCH v4 0/3] vhost-user: Add the ability to control ownership/permissions

Aaron Conole 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.

Okay.  

> 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.
>
> Daniele
>
> 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`
>  calls).
>
>  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
>  https://www.usenix.org/legacy/event/fast08/tech/full_papers/tsafrir/tsafrir_html/index.html,
>  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
>    systems.
>
>  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
>
>  --
>  2.5.5
>
>  _______________________________________________
>  dev mailing list
>  dev at openvswitch.org
>  http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list