From d077a24f5c0bef70b9cf5b11276f364dab930a64 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Sat, 1 Nov 2025 08:20:46 +0800 Subject: [PATCH] fix(socket): avoid stack-use-after-return after timeout (#36405) Problem: In socket_connect(), if connecting to the given TCP address times out, libuv is still trying to connect to the address, and connect_cb may be called when running the libuv event loop after the `status` variable referenced by `req.data` goes out of scope. Solution: Close the uv_tcp_t handle and wait for connect_cb to be called before retrying or failing in socket_connect(). This also avoid leaking libuv handles. The tests added here only check that the non-timeout case still works, as checking the timeout case is very hard without modifications to the code. Removing the first LOOP_PROCESS_EVENT_UNTIL() in socket_connect() (the one with the timeout) is a way to check that manually. Also add a comment about the cause of the ASAN error in #34586. --- src/nvim/event/socket.c | 25 +++++++++++++++++++------ src/nvim/event/stream.c | 2 ++ test/functional/core/channels_spec.lua | 16 ++++++++++++++++ 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/nvim/event/socket.c b/src/nvim/event/socket.c index 835a2b95a7..4cc7470115 100644 --- a/src/nvim/event/socket.c +++ b/src/nvim/event/socket.c @@ -190,8 +190,9 @@ static void connect_cb(uv_connect_t *req, int status) { int *ret_status = req->data; *ret_status = status; - if (status != 0) { - uv_close((uv_handle_t *)req->handle, NULL); + uv_handle_t *handle = (uv_handle_t *)req->handle; + if (status != 0 && !uv_is_closing(handle)) { + uv_close(handle, NULL); } } @@ -245,11 +246,23 @@ tcp_retry: if (status == 0) { stream_init(NULL, &stream->s, -1, uv_stream); success = true; - } else if (is_tcp && addrinfo->ai_next) { - addrinfo = addrinfo->ai_next; - goto tcp_retry; } else { - *error = _("connection refused"); + if (!uv_is_closing((uv_handle_t *)uv_stream)) { + uv_close((uv_handle_t *)uv_stream, NULL); + if (status == 1) { + // The uv_close() above will make libuv call connect_cb() with UV_ECANCELED. + // Make sure connect_cb() has been called here, as if it's called after this + // function ends it will cause a stack-use-after-scope. + LOOP_PROCESS_EVENTS_UNTIL(&main_loop, NULL, -1, status != 1); + } + } + + if (is_tcp && addrinfo->ai_next) { + addrinfo = addrinfo->ai_next; + goto tcp_retry; + } else { + *error = _("connection refused"); + } } cleanup: diff --git a/src/nvim/event/stream.c b/src/nvim/event/stream.c index 1f3ab40000..bbe616b59f 100644 --- a/src/nvim/event/stream.c +++ b/src/nvim/event/stream.c @@ -142,6 +142,8 @@ void stream_close_handle(Stream *stream) static void close_cb(uv_handle_t *handle) { Stream *stream = handle->data; + // Need to check if handle->data is NULL here as this callback may be called between + // the handle's initialization and stream_init() (e.g. in socket_connect()). if (stream && stream->close_cb) { stream->close_cb(stream, stream->close_cb_data); } diff --git a/test/functional/core/channels_spec.lua b/test/functional/core/channels_spec.lua index 11617905dd..c26f4b0fea 100644 --- a/test/functional/core/channels_spec.lua +++ b/test/functional/core/channels_spec.lua @@ -465,6 +465,22 @@ describe('channels', function() eq(true, exec_lua('return _G.result')) end) end) + + describe('sockconnect() reports error when connection fails', function() + it('in "pipe" mode', function() + eq( + 'Vim:connection failed: connection refused', + pcall_err(fn.sockconnect, 'pipe', n.new_pipename()) + ) + end) + + it('in "tcp" mode', function() + eq( + 'Vim:connection failed: connection refused', + pcall_err(fn.sockconnect, 'pipe', '127.0.0.1:0') + ) + end) + end) end) describe('loopback', function()