[PATCH net 0/2] ovpn: fix TCP teardown UAF races

[David Carlier <devnexen@gmail.com>]

Two distinct UAFs in the TCP transport teardown path, both inherited
  from 11851cbd60ea ("ovpn: implement TCP transport"). They share a race
  class with the already-merged 94560267d6c4 ("ovpn: tcp - don't deref
  NULL sk_socket member after tcp_close()"), which fixed the detach-side
  of the keepalive-vs-userspace-close race window; the two patches in
  this series cover the other two victim sites in the same window.

  Patch 1 fixes a UAF in ovpn_tcp_close(). The function loads the
  ovpn_socket via rcu_dereference_sk_user_data(), caches sock->peer in a
  local, drops rcu_read_lock, and then passes sock->peer (rather than
  the cached local) to ovpn_peer_del(). Unlike ovpn_tcp_sendmsg() which
  uses the same pattern but is protected by lock_sock, ovpn_tcp_close()
  runs without the socket lock - inet_release() does not lock_sock
  before calling sk_prot->close. A concurrent ovpn_socket_release() can
  therefore complete kref_put -> detach -> synchronize_rcu -> kfree(sock)
  in the window after rcu_read_unlock() but before the dangling
  sock->peer dereference. The cached peer local already exists, is
  held by ovpn_peer_hold() taken under RCU, and is the correct argument.

  Patch 2 fixes the CMD_NEW_PEER error path in ovpn_nl_peer_new_doit(),
  which calls ovpn_peer_release() directly rather than ovpn_peer_put(),
  bypassing the kref. The accompanying "peer was not yet hashed, thus
  not used in any context" comment is correct for UDP - whose
  ovpn_socket union uses the .ovpn arm and is unreachable from a peer
  pointer - but wrong for TCP. After ovpn_tcp_socket_attach() publishes
  ovpn_sock via rcu_assign_sk_user_data(), the peer is reachable via
  sk_user_data -> ovpn_sock->peer; userspace recvmsg/sendmsg/close/poll
  and the strparser-driven ovpn_tcp_rcv() path can bump its refcount.
  ovpn_tcp_socket_wait_finish() drains strparser and tx work but does
  not synchronize with userspace syscall callers. Use ovpn_peer_put()
  so the kref correctly defers destruction until the last reference
  is dropped.

  Reachability is narrower for patch 2 than patch 1: it requires a
  userspace operation on the TCP fd to be in flight while the netlink
  CMD_NEW_PEER handler hits an error in ovpn_nl_peer_modify() or
  ovpn_peer_add(). A well-behaved openvpn daemon issues CMD_NEW_PEER
  before passing the fd to a recv loop, but the kernel cannot enforce
  that ordering, and the fix is one line.

  Both Fixes: 11851cbd60ea ("ovpn: implement TCP transport"). Not Cc'd
  to stable - borderline practical reachability and reproducer is
  non-trivial; maintainer judgement on backport.


David Carlier (2):
  ovpn: tcp - use cached peer pointer in ovpn_tcp_close()
  ovpn: respect peer refcount in CMD_NEW_PEER error path

 drivers/net/ovpn/netlink.c | 8 +++++---
 drivers/net/ovpn/tcp.c     | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

-- 
2.53.0