]> git.ipfire.org Git - thirdparty/knot-resolver.git/commitdiff
daemon/io_create(): move session2_new_io out of io_create() docs-session-segv-qm4yug/deployments/8413
authorFrantisek Tobias <frantisek.tobias@nic.cz>
Wed, 21 Jan 2026 12:47:18 +0000 (13:47 +0100)
committerFrantisek Tobias <frantisek.tobias@nic.cz>
Wed, 21 Jan 2026 13:32:03 +0000 (14:32 +0100)
The motivation for this change is to unite the out parameter type to uv_handle_t

NEWS
daemon/io.c
daemon/io.h
daemon/worker.c

diff --git a/NEWS b/NEWS
index 823517bfff3e5d647476c616f743107f88b09eda..0dcdbc78b82ac6d60ec8a3767078fe33b2ec0354 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -28,6 +28,7 @@ Bugfixes
 - reload did not apply changes to /fallback (!1763)
 - fix config.cache.clear test on apple silicon (!1766)
 - cache: fix wrong TTL in some cases, typically 32768 (!1774)
+- fix handling of protolayer_data_sess_init_cb failure (!1797)
 
 
 Knot Resolver 6.0.17 (2025-12-02)
index 487e593803778edded1fc9d0330e8a54ac8c1117..2f06f1a8ebd38a5c2ef1aed6eb59f8e51f18ebad 100644 (file)
@@ -381,51 +381,41 @@ static void tcp_accept_internal(uv_stream_t *master, int status, enum kr_proto g
                return;
        }
 
-       union session_or_handle out = { 0 };
-       int res = io_create(master->loop, &out, SOCK_STREAM, AF_UNSPEC, grp,
-                       NULL, 0, false);
-       if (res) {
-               if (res == UV_EMFILE) {
-                       the_worker->too_many_open = true;
-                       the_worker->rconcurrent_highwatermark = the_worker->stats.rconcurrent;
-               }
-               if (out.handle != NULL) {
+       uv_handle_t *client = { 0 };
+       if (io_create(master->loop, &client, SOCK_STREAM, AF_UNSPEC)) {
+               return;
+       }
+
+       struct session2 *s = session2_new_io(client, grp, NULL, 0, false);
+       int res = uv_accept(master, (uv_stream_t *)client);
+       if (s == NULL || res) {
+               if (s == NULL) {
                        /* Since res isn't OK struct session wasn't
                         * allocated \ borrowed. We must release client handle
                         * only. But first accept the connection, as it has
                         * already been established by the kernel and
-                        * it is required for proper termination.
-                        */
-                       if (uv_accept(master, (uv_stream_t *)out.handle) == 0) {
-                               uv_close(out.handle, (uv_close_cb)free);
-                       }
+                        * it is required for proper termination. */
+                       uv_close(client, (uv_close_cb)free);
+               } else {
+                       /* close session, close underlying uv handles and
+                        * deallocate (or return to memory pool) memory. */
+                       session2_close(s);
                }
                return;
        }
 
-       struct session2 *s = out.session;
-       kr_require(s->outgoing == false);
-
-       uv_tcp_t *client = (uv_tcp_t *)session2_get_handle(s);
-       if (uv_accept(master, (uv_stream_t *)client) != 0) {
-               /* close session, close underlying uv handles and
-                * deallocate (or return to memory pool) memory. */
-               session2_close(s);
-               return;
-       }
-
        /* Get peer's and our address.  We apparently get specific sockname here
         * even if we listened on a wildcard address. */
        struct sockaddr *sa = session2_get_peer(s);
        int sa_len = sizeof(struct sockaddr_in6);
-       int ret = uv_tcp_getpeername(client, sa, &sa_len);
+       int ret = uv_tcp_getpeername((uv_tcp_t *)client, sa, &sa_len);
        if (ret || sa->sa_family == AF_UNSPEC) {
                session2_close(s);
                return;
        }
        sa = session2_get_sockname(s);
        sa_len = sizeof(struct sockaddr_in6);
-       ret = uv_tcp_getsockname(client, sa, &sa_len);
+       ret = uv_tcp_getsockname((uv_tcp_t *)client, sa, &sa_len);
        if (ret || sa->sa_family == AF_UNSPEC) {
                session2_close(s);
                return;
@@ -440,7 +430,7 @@ static void tcp_accept_internal(uv_stream_t *master, int status, enum kr_proto g
        session2_event(s, PROTOLAYER_EVENT_CONNECT, NULL);
        session2_timer_start(s, PROTOLAYER_EVENT_GENERAL_TIMEOUT,
                        timeout, idle_in_timeout);
-       io_start_read((uv_handle_t *)client);
+       io_start_read(client);
 }
 
 static void tcp_accept(uv_stream_t *master, int status)
@@ -925,41 +915,35 @@ int io_listen_xdp(uv_loop_t *loop, struct endpoint *ep, const char *ifname)
 }
 #endif
 
-int io_create(uv_loop_t *loop, union session_or_handle *out,
-              int type, unsigned family, enum kr_proto grp,
-              struct protolayer_data_param *layer_param,
-              size_t layer_param_count, bool outgoing)
+int io_create(uv_loop_t *loop, uv_handle_t **handle,
+              int type, unsigned family)
 {
-       out->session = NULL;
+       *handle = NULL;
        int ret = -1;
-       uv_handle_t *handle;
        if (type == SOCK_DGRAM) {
                uv_udp_t *udp = malloc(sizeof(uv_udp_t));
                kr_require(udp);
                ret = uv_udp_init(loop, udp);
 
-               handle = (uv_handle_t *)udp;
+               *handle = (uv_handle_t *)udp;
        } else if (type == SOCK_STREAM) {
                uv_tcp_t *tcp = malloc(sizeof(uv_tcp_t));
                kr_require(tcp);
                ret = uv_tcp_init_ex(loop, tcp, family);
                uv_tcp_nodelay(tcp, 1);
 
-               handle = (uv_handle_t *)tcp;
+               *handle = (uv_handle_t *)tcp;
        } else {
                kr_require(false && "io_create: invalid socket type");
        }
-       if (ret != 0) {
-               return ret;
+       if (ret != 0 && *handle) {
+               free(*handle);
        }
-       struct session2 *s = session2_new_io(handle, grp, layer_param,
-                       layer_param_count, outgoing);
-       if (unlikely(s == NULL)) {
-               out->handle = handle;
-               return -1;
+       if (ret == UV_EMFILE) {
+               the_worker->too_many_open = true;
+               the_worker->rconcurrent_highwatermark = the_worker->stats.rconcurrent;
        }
 
-       out->session = s;
        return ret;
 }
 
index 2adcffd12258ddf81f50cfec983bf972da7ff465..0d036764d4cf7bda2d734d9245915f6b8ff8e15b 100644 (file)
 struct tls_ctx;
 struct tls_client_ctx;
 struct io_stream_data;
-/* union used for io_create. handle will be used only if session init fails,
- * allowing us to terminated the uv_handle gracefully */
-union session_or_handle {
-       struct session2 *session;
-       uv_handle_t *handle;
-};
 
 /** Bind address into a file-descriptor (only, no libuv).  type is e.g. SOCK_DGRAM */
 int io_bind(const struct sockaddr *addr, int type, const endpoint_flags_t *flags);
@@ -43,14 +37,11 @@ struct io_stream_data *io_tty_alloc_data(void);
 
 void tcp_timeout_trigger(uv_timer_t *timer);
 
-/** Initialize the handle, incl. ->data = struct session * instance.
+/** Initialize the handle
  * \param type = SOCK_*
  * \param family = AF_*
  * \param has_tls has meanings only when type is SOCK_STREAM */
-int io_create(uv_loop_t *loop, union session_or_handle *out, int type,
-              unsigned family, enum kr_proto grp,
-              struct protolayer_data_param *layer_param,
-              size_t layer_param_count, bool outgoing);
+int io_create(uv_loop_t *loop, uv_handle_t **handle, int type, unsigned family);
 void io_free(uv_handle_t *handle);
 
 int io_start_read(uv_handle_t *handle);
index 9bbc0607994206bc4d4284b625fa2757f7e0aee4..31774aef990a66bc208652657c5299a9d79712a8 100644 (file)
@@ -157,17 +157,15 @@ static struct session2 *ioreq_spawn(int socktype, sa_family_t family,
        }
 
        /* Create connection for iterative query */
-       union session_or_handle out;
-       int ret = io_create(the_worker->loop, &out, socktype, family, grp,
-                       layer_param, layer_param_count, true);
-       if (ret) {
-               if (ret == UV_EMFILE) {
-                       the_worker->too_many_open = true;
-                       the_worker->rconcurrent_highwatermark = the_worker->stats.rconcurrent;
-               }
+       uv_handle_t *handle = { 0 };
+       if (io_create(the_worker->loop, &handle, socktype, family)) {
+               return NULL;
+       }
+       struct session2 *s = session2_new_io(handle, grp, layer_param,
+                       layer_param_count, true);
+       if (!s) {
                return NULL;
        }
-       struct session2 *s = out.session;
 
        /* Bind to outgoing address, according to IP v4/v6. */
        union kr_sockaddr *addr;
@@ -176,6 +174,7 @@ static struct session2 *ioreq_spawn(int socktype, sa_family_t family,
        } else {
                addr = (union kr_sockaddr *)&the_worker->out_addr6;
        }
+       int ret = 0;
        if (addr->ip.sa_family != AF_UNSPEC) {
                if (kr_fails_assert(addr->ip.sa_family == family)) {
                        session2_force_close(s);