From d796bedf5ba0e4c5ed431257836692adc6c19349 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= Date: Tue, 16 Aug 2016 00:15:54 +0200 Subject: [PATCH] unix,win: make on_alloc_cb failures more resilient Treat both the base being NULL or the length being 0 as ENOBUFS. PR-URL: https://github.com/libuv/libuv/pull/997 Reviewed-By: Ben Noordhuis --- Makefile.am | 2 + src/unix/stream.c | 3 +- src/unix/udp.c | 3 +- src/win/pipe.c | 3 +- src/win/tcp.c | 7 +- src/win/tty.c | 7 +- src/win/udp.c | 6 +- test/test-list.h | 5 + test/test-tcp-alloc-cb-fail.c | 123 +++++++++++++++++++ test/test-tcp-write-queue-order.c | 4 +- test/test-udp-alloc-cb-fail.c | 197 ++++++++++++++++++++++++++++++ uv.gyp | 2 + 12 files changed, 351 insertions(+), 11 deletions(-) create mode 100644 test/test-tcp-alloc-cb-fail.c create mode 100644 test/test-udp-alloc-cb-fail.c diff --git a/Makefile.am b/Makefile.am index 5462aca5..a0437359 100644 --- a/Makefile.am +++ b/Makefile.am @@ -216,6 +216,7 @@ test_run_tests_SOURCES = test/blackhole-server.c \ test/test-socket-buffer-size.c \ test/test-spawn.c \ test/test-stdio-over-pipes.c \ + test/test-tcp-alloc-cb-fail.c \ test/test-tcp-bind-error.c \ test/test-tcp-bind6-error.c \ test/test-tcp-close-accept.c \ @@ -247,6 +248,7 @@ test_run_tests_SOURCES = test/blackhole-server.c \ test/test-timer.c \ test/test-tmpdir.c \ test/test-tty.c \ + test/test-udp-alloc-cb-fail.c \ test/test-udp-bind.c \ test/test-udp-create-socket-early.c \ test/test-udp-dgram-too-big.c \ diff --git a/src/unix/stream.c b/src/unix/stream.c index d0c2f1ad..5dc2f83f 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -1124,8 +1124,9 @@ static void uv__read(uv_stream_t* stream) { && (count-- > 0)) { assert(stream->alloc_cb != NULL); + buf = uv_buf_init(NULL, 0); stream->alloc_cb((uv_handle_t*)stream, 64 * 1024, &buf); - if (buf.len == 0) { + if (buf.base == NULL || buf.len == 0) { /* User indicates it can't or won't handle the read. */ stream->read_cb(stream, UV_ENOBUFS, &buf); return; diff --git a/src/unix/udp.c b/src/unix/udp.c index 4527bba1..f5d7ed14 100644 --- a/src/unix/udp.c +++ b/src/unix/udp.c @@ -165,8 +165,9 @@ static void uv__udp_recvmsg(uv_udp_t* handle) { h.msg_name = &peer; do { + buf = uv_buf_init(NULL, 0); handle->alloc_cb((uv_handle_t*) handle, 64 * 1024, &buf); - if (buf.len == 0) { + if (buf.base == NULL || buf.len == 0) { handle->recv_cb(handle, UV_ENOBUFS, &buf, NULL, 0); return; } diff --git a/src/win/pipe.c b/src/win/pipe.c index 2a949e79..2442be73 100644 --- a/src/win/pipe.c +++ b/src/win/pipe.c @@ -1634,8 +1634,9 @@ void uv_process_pipe_read_req(uv_loop_t* loop, uv_pipe_t* handle, } } + buf = uv_buf_init(NULL, 0); handle->alloc_cb((uv_handle_t*) handle, avail, &buf); - if (buf.len == 0) { + if (buf.base == NULL || buf.len == 0) { handle->read_cb((uv_stream_t*) handle, UV_ENOBUFS, &buf); break; } diff --git a/src/win/tcp.c b/src/win/tcp.c index 0f565486..0709696f 100644 --- a/src/win/tcp.c +++ b/src/win/tcp.c @@ -496,8 +496,10 @@ static void uv_tcp_queue_read(uv_loop_t* loop, uv_tcp_t* handle) { */ if (loop->active_tcp_streams < uv_active_tcp_streams_threshold) { handle->flags &= ~UV_HANDLE_ZERO_READ; + handle->tcp.conn.read_buffer = uv_buf_init(NULL, 0); handle->alloc_cb((uv_handle_t*) handle, 65536, &handle->tcp.conn.read_buffer); - if (handle->tcp.conn.read_buffer.len == 0) { + if (handle->tcp.conn.read_buffer.base == NULL || + handle->tcp.conn.read_buffer.len == 0) { handle->read_cb((uv_stream_t*) handle, UV_ENOBUFS, &handle->tcp.conn.read_buffer); return; } @@ -1004,8 +1006,9 @@ void uv_process_tcp_read_req(uv_loop_t* loop, uv_tcp_t* handle, /* Do nonblocking reads until the buffer is empty */ while (handle->flags & UV_HANDLE_READING) { + buf = uv_buf_init(NULL, 0); handle->alloc_cb((uv_handle_t*) handle, 65536, &buf); - if (buf.len == 0) { + if (buf.base == NULL || buf.len == 0) { handle->read_cb((uv_stream_t*) handle, UV_ENOBUFS, &buf); break; } diff --git a/src/win/tty.c b/src/win/tty.c index daa303f1..9bb7879d 100644 --- a/src/win/tty.c +++ b/src/win/tty.c @@ -509,8 +509,10 @@ static void uv_tty_queue_read_line(uv_loop_t* loop, uv_tty_t* handle) { req = &handle->read_req; memset(&req->u.io.overlapped, 0, sizeof(req->u.io.overlapped)); + handle->tty.rd.read_line_buffer = uv_buf_init(NULL, 0); handle->alloc_cb((uv_handle_t*) handle, 8192, &handle->tty.rd.read_line_buffer); - if (handle->tty.rd.read_line_buffer.len == 0) { + if (handle->tty.rd.read_line_buffer.base == NULL || + handle->tty.rd.read_line_buffer.len == 0) { handle->read_cb((uv_stream_t*) handle, UV_ENOBUFS, &handle->tty.rd.read_line_buffer); @@ -833,8 +835,9 @@ void uv_process_tty_read_raw_req(uv_loop_t* loop, uv_tty_t* handle, if (handle->tty.rd.last_key_offset < handle->tty.rd.last_key_len) { /* Allocate a buffer if needed */ if (buf_used == 0) { + buf = uv_buf_init(NULL, 0); handle->alloc_cb((uv_handle_t*) handle, 1024, &buf); - if (buf.len == 0) { + if (buf.base == NULL || buf.len == 0) { handle->read_cb((uv_stream_t*) handle, UV_ENOBUFS, &buf); goto out; } diff --git a/src/win/udp.c b/src/win/udp.c index 24792ec0..9bf1453e 100644 --- a/src/win/udp.c +++ b/src/win/udp.c @@ -289,8 +289,9 @@ static void uv_udp_queue_recv(uv_loop_t* loop, uv_udp_t* handle) { if (loop->active_udp_streams < uv_active_udp_streams_threshold) { handle->flags &= ~UV_HANDLE_ZERO_READ; + handle->recv_buffer = uv_buf_init(NULL, 0); handle->alloc_cb((uv_handle_t*) handle, 65536, &handle->recv_buffer); - if (handle->recv_buffer.len == 0) { + if (handle->recv_buffer.base == NULL || handle->recv_buffer.len == 0) { handle->recv_cb(handle, UV_ENOBUFS, &handle->recv_buffer, NULL, 0); return; } @@ -506,8 +507,9 @@ void uv_process_udp_recv_req(uv_loop_t* loop, uv_udp_t* handle, /* Do a nonblocking receive */ /* TODO: try to read multiple datagrams at once. FIONREAD maybe? */ + buf = uv_buf_init(NULL, 0); handle->alloc_cb((uv_handle_t*) handle, 65536, &buf); - if (buf.len == 0) { + if (buf.base == NULL || buf.len == 0) { handle->recv_cb(handle, UV_ENOBUFS, &buf, NULL, 0); goto done; } diff --git a/test/test-list.h b/test/test-list.h index 41ed64c6..2f19b327 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -61,6 +61,7 @@ TEST_DECLARE (ipc_send_recv_pipe_inprocess) TEST_DECLARE (ipc_send_recv_tcp) TEST_DECLARE (ipc_send_recv_tcp_inprocess) TEST_DECLARE (ipc_tcp_connection) +TEST_DECLARE (tcp_alloc_cb_fail) TEST_DECLARE (tcp_ping_pong) TEST_DECLARE (tcp_ping_pong_v6) TEST_DECLARE (pipe_ping_pong) @@ -106,6 +107,7 @@ TEST_DECLARE (tcp_bind6_error_addrnotavail) TEST_DECLARE (tcp_bind6_error_fault) TEST_DECLARE (tcp_bind6_error_inval) TEST_DECLARE (tcp_bind6_localhost_ok) +TEST_DECLARE (udp_alloc_cb_fail) TEST_DECLARE (udp_bind) TEST_DECLARE (udp_bind_reuseaddr) TEST_DECLARE (udp_create_early) @@ -416,6 +418,8 @@ TASK_LIST_START TEST_ENTRY (ipc_send_recv_tcp_inprocess) TEST_ENTRY (ipc_tcp_connection) + TEST_ENTRY (tcp_alloc_cb_fail) + TEST_ENTRY (tcp_ping_pong) TEST_HELPER (tcp_ping_pong, tcp4_echo_server) @@ -483,6 +487,7 @@ TASK_LIST_START TEST_ENTRY (tcp_bind6_error_inval) TEST_ENTRY (tcp_bind6_localhost_ok) + TEST_ENTRY (udp_alloc_cb_fail) TEST_ENTRY (udp_bind) TEST_ENTRY (udp_bind_reuseaddr) TEST_ENTRY (udp_create_early) diff --git a/test/test-tcp-alloc-cb-fail.c b/test/test-tcp-alloc-cb-fail.c new file mode 100644 index 00000000..61ca667a --- /dev/null +++ b/test/test-tcp-alloc-cb-fail.c @@ -0,0 +1,123 @@ +/* Copyright libuv project and contributors. All rights reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include +#include +#include + +#include "uv.h" +#include "task.h" + +static uv_tcp_t server; +static uv_tcp_t client; +static uv_tcp_t incoming; +static int connect_cb_called; +static int close_cb_called; +static int connection_cb_called; +static uv_write_t write_req; + +static char hello[] = "HELLO!"; + + +static void close_cb(uv_handle_t* handle) { + close_cb_called++; +} + +static void write_cb(uv_write_t* req, int status) { + ASSERT(status == 0); +} + +static void conn_alloc_cb(uv_handle_t* handle, size_t size, uv_buf_t* buf) { + /* Do nothing, read_cb should be called with UV_ENOBUFS. */ +} + +static void conn_read_cb(uv_stream_t* stream, + ssize_t nread, + const uv_buf_t* buf) { + ASSERT(nread == UV_ENOBUFS); + ASSERT(buf->base == NULL); + ASSERT(buf->len == 0); + + uv_close((uv_handle_t*) &incoming, close_cb); + uv_close((uv_handle_t*) &client, close_cb); + uv_close((uv_handle_t*) &server, close_cb); +} + +static void connect_cb(uv_connect_t* req, int status) { + int r; + uv_buf_t buf; + + ASSERT(status == 0); + connect_cb_called++; + + buf = uv_buf_init(hello, sizeof(hello)); + r = uv_write(&write_req, req->handle, &buf, 1, write_cb); + ASSERT(r == 0); +} + + +static void connection_cb(uv_stream_t* tcp, int status) { + ASSERT(status == 0); + + ASSERT(0 == uv_tcp_init(tcp->loop, &incoming)); + ASSERT(0 == uv_accept(tcp, (uv_stream_t*) &incoming)); + ASSERT(0 == uv_read_start((uv_stream_t*) &incoming, + conn_alloc_cb, + conn_read_cb)); + + connection_cb_called++; +} + + +static void start_server(void) { + struct sockaddr_in addr; + + ASSERT(0 == uv_ip4_addr("0.0.0.0", TEST_PORT, &addr)); + + ASSERT(0 == uv_tcp_init(uv_default_loop(), &server)); + ASSERT(0 == uv_tcp_bind(&server, (struct sockaddr*) &addr, 0)); + ASSERT(0 == uv_listen((uv_stream_t*) &server, 128, connection_cb)); +} + + +TEST_IMPL(tcp_alloc_cb_fail) { + uv_connect_t connect_req; + struct sockaddr_in addr; + + start_server(); + + ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT, &addr)); + + ASSERT(0 == uv_tcp_init(uv_default_loop(), &client)); + ASSERT(0 == uv_tcp_connect(&connect_req, + &client, + (struct sockaddr*) &addr, + connect_cb)); + + ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_DEFAULT)); + + ASSERT(connect_cb_called == 1); + ASSERT(connection_cb_called == 1); + ASSERT(close_cb_called == 3); + + MAKE_VALGRIND_HAPPY(); + return 0; +} diff --git a/test/test-tcp-write-queue-order.c b/test/test-tcp-write-queue-order.c index 8a98ab83..d50289c3 100644 --- a/test/test-tcp-write-queue-order.c +++ b/test/test-tcp-write-queue-order.c @@ -46,13 +46,13 @@ static void close_cb(uv_handle_t* handle) { close_cb_called++; } -void timer_cb(uv_timer_t* handle) { +static void timer_cb(uv_timer_t* handle) { uv_close((uv_handle_t*) &client, close_cb); uv_close((uv_handle_t*) &server, close_cb); uv_close((uv_handle_t*) &incoming, close_cb); } -void write_cb(uv_write_t* req, int status) { +static void write_cb(uv_write_t* req, int status) { if (status == 0) write_callbacks++; else if (status == UV_ECANCELED) diff --git a/test/test-udp-alloc-cb-fail.c b/test/test-udp-alloc-cb-fail.c new file mode 100644 index 00000000..05b871e9 --- /dev/null +++ b/test/test-udp-alloc-cb-fail.c @@ -0,0 +1,197 @@ +/* Copyright libuv project and other Node contributors. All rights reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include "uv.h" +#include "task.h" + +#include +#include +#include + +#define CHECK_HANDLE(handle) \ + ASSERT((uv_udp_t*)(handle) == &server || (uv_udp_t*)(handle) == &client) + +static uv_udp_t server; +static uv_udp_t client; + +static int cl_send_cb_called; +static int cl_recv_cb_called; + +static int sv_send_cb_called; +static int sv_recv_cb_called; + +static int close_cb_called; + + +static void sv_alloc_cb(uv_handle_t* handle, + size_t suggested_size, + uv_buf_t* buf) { + static char slab[65536]; + CHECK_HANDLE(handle); + buf->base = slab; + buf->len = sizeof(slab); +} + + +static void cl_alloc_cb(uv_handle_t* handle, + size_t suggested_size, + uv_buf_t* buf) { + /* Do nothing, recv_cb should be called with UV_ENOBUFS. */ +} + + +static void close_cb(uv_handle_t* handle) { + CHECK_HANDLE(handle); + ASSERT(1 == uv_is_closing(handle)); + close_cb_called++; +} + + +static void cl_recv_cb(uv_udp_t* handle, + ssize_t nread, + const uv_buf_t* buf, + const struct sockaddr* addr, + unsigned flags) { + CHECK_HANDLE(handle); + ASSERT(flags == 0); + ASSERT(nread == UV_ENOBUFS); + + cl_recv_cb_called++; + + uv_close((uv_handle_t*) handle, close_cb); +} + + +static void cl_send_cb(uv_udp_send_t* req, int status) { + int r; + + ASSERT(req != NULL); + ASSERT(status == 0); + CHECK_HANDLE(req->handle); + + r = uv_udp_recv_start(req->handle, cl_alloc_cb, cl_recv_cb); + ASSERT(r == 0); + + cl_send_cb_called++; +} + + +static void sv_send_cb(uv_udp_send_t* req, int status) { + ASSERT(req != NULL); + ASSERT(status == 0); + CHECK_HANDLE(req->handle); + + uv_close((uv_handle_t*) req->handle, close_cb); + free(req); + + sv_send_cb_called++; +} + + +static void sv_recv_cb(uv_udp_t* handle, + ssize_t nread, + const uv_buf_t* rcvbuf, + const struct sockaddr* addr, + unsigned flags) { + uv_udp_send_t* req; + uv_buf_t sndbuf; + int r; + + if (nread < 0) { + ASSERT(0 && "unexpected error"); + } + + if (nread == 0) { + /* Returning unused buffer */ + /* Don't count towards sv_recv_cb_called */ + ASSERT(addr == NULL); + return; + } + + CHECK_HANDLE(handle); + ASSERT(flags == 0); + + ASSERT(addr != NULL); + ASSERT(nread == 4); + ASSERT(!memcmp("PING", rcvbuf->base, nread)); + + r = uv_udp_recv_stop(handle); + ASSERT(r == 0); + + req = malloc(sizeof *req); + ASSERT(req != NULL); + + sndbuf = uv_buf_init("PONG", 4); + r = uv_udp_send(req, handle, &sndbuf, 1, addr, sv_send_cb); + ASSERT(r == 0); + + sv_recv_cb_called++; +} + + +TEST_IMPL(udp_alloc_cb_fail) { + struct sockaddr_in addr; + uv_udp_send_t req; + uv_buf_t buf; + int r; + + ASSERT(0 == uv_ip4_addr("0.0.0.0", TEST_PORT, &addr)); + + r = uv_udp_init(uv_default_loop(), &server); + ASSERT(r == 0); + + r = uv_udp_bind(&server, (const struct sockaddr*) &addr, 0); + ASSERT(r == 0); + + r = uv_udp_recv_start(&server, sv_alloc_cb, sv_recv_cb); + ASSERT(r == 0); + + ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT, &addr)); + + r = uv_udp_init(uv_default_loop(), &client); + ASSERT(r == 0); + + buf = uv_buf_init("PING", 4); + r = uv_udp_send(&req, + &client, + &buf, + 1, + (const struct sockaddr*) &addr, + cl_send_cb); + ASSERT(r == 0); + + ASSERT(close_cb_called == 0); + ASSERT(cl_send_cb_called == 0); + ASSERT(cl_recv_cb_called == 0); + ASSERT(sv_send_cb_called == 0); + ASSERT(sv_recv_cb_called == 0); + + uv_run(uv_default_loop(), UV_RUN_DEFAULT); + + ASSERT(cl_send_cb_called == 1); + ASSERT(cl_recv_cb_called == 1); + ASSERT(sv_send_cb_called == 1); + ASSERT(sv_recv_cb_called == 1); + ASSERT(close_cb_called == 2); + + MAKE_VALGRIND_HAPPY(); + return 0; +} diff --git a/uv.gyp b/uv.gyp index b83f82b0..953a6fdc 100644 --- a/uv.gyp +++ b/uv.gyp @@ -364,6 +364,7 @@ 'test/test-spawn.c', 'test/test-fs-poll.c', 'test/test-stdio-over-pipes.c', + 'test/test-tcp-alloc-cb-fail.c', 'test/test-tcp-bind-error.c', 'test/test-tcp-bind6-error.c', 'test/test-tcp-close.c', @@ -398,6 +399,7 @@ 'test/test-timer-from-check.c', 'test/test-timer.c', 'test/test-tty.c', + 'test/test-udp-alloc-cb-fail.c', 'test/test-udp-bind.c', 'test/test-udp-create-socket-early.c', 'test/test-udp-dgram-too-big.c',