hvs_close_lock_held() takes a scheduling reference (sock_hold()) and
schedules hvs_close_timeout(); that reference is dropped, with
vsock_remove_sock(), by the delayed close.
hvs_do_close_lock_held(vsk, cancel_timeout=true) drops the reference and
removes the socket only if cancel_delayed_work() succeeds. When the host
rescind callback hvs_close_connection() runs while hvs_close_timeout() is
already dequeued and blocked on lock_sock(), cancel_delayed_work() returns
false: the reference is not dropped and the socket is not removed, only
SOCK_DONE is set. hvs_close_timeout() then sees SOCK_DONE, skips
hvs_do_close_lock_held(), and drops only its own local reference -- the
scheduling reference leaks and the socket is never removed from the
bound/connected tables.
Make the running timeout complete the cleanup the rescind path could not:
when SOCK_DONE is set but the work is still marked scheduled, drop the
scheduling reference and remove the socket. The two cleanup sites are
mutually exclusive and guarded by close_work_scheduled, so the reference
is dropped exactly once. (cancel_delayed_work_sync() cannot be used from
hvs_do_close_lock_held(): it runs under the same lock_sock() the work
takes.)
Signed-off-by: Bartłomiej Dmitruk <redacted>
---
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -499,10 +499,18 @@
sock_hold(sk);
lock_sock(sk);
- if (!sock_flag(sk, SOCK_DONE))
+ if (!sock_flag(sk, SOCK_DONE)) {
hvs_do_close_lock_held(vsk, false);
-
- vsk->close_work_scheduled = false;
+ } else if (vsk->close_work_scheduled) {
+ /* A concurrent rescind (hvs_close_connection) set SOCK_DONE but
+ * could not cancel this already-running work, so it left the
+ * scheduling reference and vsock_remove_sock() to us. Finish
+ * the cleanup to avoid leaking the socket and its table entry.
+ */
+ vsk->close_work_scheduled = false;
+ vsock_remove_sock(vsk);
+ sock_put(sk);
+ }
release_sock(sk);
sock_put(sk);
}