From: Gianmarco De Gregori Date: Tue, 16 Jun 2026 16:21:23 +0000 (+0200) Subject: Multisocket: use event engine rwflags for UDP I/O X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1281d264a259bf8e95695bbb831737a4778d4afa;p=thirdparty%2Fopenvpn.git Multisocket: use event engine rwflags for UDP I/O udp_flags does not guarantee correct association with the socket being processed. Use the rwflags delivered by the event engine along with the event to ensure proper per-socket I/O handling. Remove udp_flags entirely. Replace the previous global-style flag computation with a per-socket decision model in p2mp_iow_flags(), which derives I/O flags directly from the current multi_context state and the specific socket being processed. This ensures that read/write decisions are correctly bound to the active socket rather than shared or implicit global state. This change is based on an investigation triggered by a report from Joshua Rogers using ZeroPath. Change-Id: I6b303805a3688b6f6363140c76853a58badecd8f Signed-off-by: Gianmarco De Gregori Acked-by: Gert Doering Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1635 Message-Id: <20260616162129.28519-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37198.html Signed-off-by: Gert Doering --- diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index d24f53468..ab76b9e00 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -2030,9 +2030,8 @@ pre_select(struct context *c) check_timeout_random_component(c); } -static void -multi_io_process_flags(struct context *c, struct event_set *es, const unsigned int flags, - unsigned int *out_socket, unsigned int *out_tuntap) +void +multi_io_process_flags(struct context *c, struct event_set *es, struct link_socket *sock, const unsigned int flags) { unsigned int socket = 0; unsigned int tuntap = 0; @@ -2123,50 +2122,19 @@ multi_io_process_flags(struct context *c, struct event_set *es, const unsigned i * (for TCP server sockets this happens in * socket_set_listen_persistent()). */ - for (int i = 0; i < c->c1.link_sockets_num; i++) - { - if ((c->options.mode != MODE_SERVER) || (proto_is_dgram(c->c2.link_sockets[i]->info.proto))) - { - socket_set(c->c2.link_sockets[i], es, socket, &c->c2.link_sockets[i]->ev_arg, NULL); - } - } - + socket_set(sock, es, socket, &sock->ev_arg, NULL); tun_set(c->c1.tuntap, es, tuntap, (void *)tun_shift, NULL); - - if (out_socket) - { - *out_socket = socket; - } - - if (out_tuntap) - { - *out_tuntap = tuntap; - } } /* - * Wait for I/O events. Used for UDP sockets in - * point-to-multipoint mode. - */ - -void -get_io_flags_udp(struct context *c, struct multi_io *multi_io, const unsigned int flags) -{ - unsigned int out_socket; - - multi_io_process_flags(c, multi_io->es, flags, &out_socket, NULL); - multi_io->udp_flags = (out_socket << SOCKET_SHIFT); -} - -/* - * This is the core I/O wait function, used for all I/O waits except - * for the top-level server sockets. + * This is the core I/O wait function, used for all I/O waits. + * + * Invoked by P2P instances in tunnel_point_to_point() or by P2MP + * instances within the per-client context in multi_io.c. */ void io_wait(struct context *c, const unsigned int flags) { - unsigned int out_socket; - unsigned int out_tuntap; struct event_set_return esr[4]; /* These shifts all depend on EVENT_READ and EVENT_WRITE */ @@ -2185,7 +2153,7 @@ io_wait(struct context *c, const unsigned int flags) */ event_reset(c->c2.event_set); - multi_io_process_flags(c, c->c2.event_set, flags, &out_socket, &out_tuntap); + multi_io_process_flags(c, c->c2.event_set, c->c2.link_sockets[0], flags); #if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) if (c->c1.tuntap) diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index 0d3e492be..324c0b4bf 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -68,7 +68,22 @@ extern counter_type link_read_bytes_global; extern counter_type link_write_bytes_global; -void get_io_flags_udp(struct context *c, struct multi_io *multi_io, const unsigned int flags); +/** + * @brief Processes I/O flags to configure socket and TUN/TAP event monitors. + * + * Evaluates the current I/O state (`flags`) and context mode to determine + * which READ or WRITE events to register in the event loop. + * + * @note This function gets called from client instances from @c io_wait() + * and from p2mp (Point-to-multipoint) instances in @c multi_io_wait(). + * + * @param c Pointer to the core context. + * @param es The event set where descriptors are registered. + * @param sock The link socket to configure. + * @param flags Bitmask of active I/O operation requests (e.g., @ref IOW_TO_LINK, + * @ref IOW_READ_TUN, @ref IOW_TO_TUN, @ref IOW_SHAPER). + */ +void multi_io_process_flags(struct context *c, struct event_set *es, struct link_socket *sock, const unsigned int flags); void io_wait(struct context *c, const unsigned int flags); diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c index dded1ee69..143a33018 100644 --- a/src/openvpn/mudp.c +++ b/src/openvpn/mudp.c @@ -340,18 +340,17 @@ multi_process_outgoing_link(struct multi_context *m, const unsigned int mpp_flag * Process a UDP socket event. */ void -multi_process_io_udp(struct multi_context *m, struct link_socket *sock) +multi_process_io_udp(struct multi_context *m, struct link_socket *sock, unsigned int rwflags) { - const unsigned int status = m->multi_io->udp_flags; const unsigned int mpp_flags = (MPP_PRE_SELECT | MPP_CLOSE_ON_SIGNAL); /* UDP port ready to accept write */ - if (status & SOCKET_WRITE) + if (rwflags & SOCKET_WRITE) { multi_process_outgoing_link(m, mpp_flags); } /* Incoming data on UDP port */ - else if (status & SOCKET_READ) + else if (rwflags & SOCKET_READ) { read_incoming_link(&m->top, sock); if (!IS_SIG(&m->top)) @@ -359,36 +358,4 @@ multi_process_io_udp(struct multi_context *m, struct link_socket *sock) multi_process_incoming_link(m, NULL, mpp_flags, sock); } } - - m->multi_io->udp_flags = ES_ERROR; -} - -/* - * Return the io_wait() flags appropriate for - * a point-to-multipoint tunnel. - */ -unsigned int -p2mp_iow_flags(const struct multi_context *m) -{ - unsigned int flags = IOW_WAIT_SIGNAL; - if (m->pending) - { - if (TUN_OUT(&m->pending->context)) - { - flags |= IOW_TO_TUN; - } - if (LINK_OUT(&m->pending->context)) - { - flags |= IOW_TO_LINK; - } - } - else if (mbuf_defined(m->mbuf)) - { - flags |= IOW_MBUF; - } - else - { - flags |= IOW_READ; - } - return flags; } diff --git a/src/openvpn/mudp.h b/src/openvpn/mudp.h index 005ee1056..65675390a 100644 --- a/src/openvpn/mudp.h +++ b/src/openvpn/mudp.h @@ -30,9 +30,7 @@ struct context; struct multi_context; -unsigned int p2mp_iow_flags(const struct multi_context *m); - -void multi_process_io_udp(struct multi_context *m, struct link_socket *sock); +void multi_process_io_udp(struct multi_context *m, struct link_socket *sock, unsigned int rwflags); /**************************************************************************/ /** * Get, and if necessary create, the multi_instance associated with a diff --git a/src/openvpn/multi_io.c b/src/openvpn/multi_io.c index e6f4e9c2e..d8cc7088d 100644 --- a/src/openvpn/multi_io.c +++ b/src/openvpn/multi_io.c @@ -154,6 +154,40 @@ multi_io_free(struct multi_io *multi_io) } } +/* + * Return the io flags appropriate for + * a point-to-multipoint tunnel. + */ +static unsigned int +p2mp_iow_flags(const struct multi_context *m, struct link_socket *sock) +{ + unsigned int flags = IOW_WAIT_SIGNAL; + + if (m->pending) + { + if (TUN_OUT(&m->pending->context)) + { + flags |= IOW_TO_TUN; + } + + /* If the to be processed socket happens to be the same one on the pending + * UDP client instance, we set that socket ready for write */ + if (LINK_OUT(&m->pending->context) && m->pending->context.c2.link_sockets[0] == sock) + { + flags |= IOW_TO_LINK; + } + } + else if (mbuf_defined(m->mbuf)) + { + flags |= IOW_MBUF; + } + else + { + flags |= IOW_READ; + } + return flags; +} + int multi_io_wait(struct multi_context *m) { @@ -171,7 +205,17 @@ multi_io_wait(struct multi_context *m) if (has_udp_in_local_list(&m->top.options)) { - get_io_flags_udp(&m->top, m->multi_io, p2mp_iow_flags(m)); + for (int i = 0; i < m->top.c1.link_sockets_num; i++) + { + struct link_socket *sock = m->top.c2.link_sockets[i]; + + if ((m->top.options.mode == MODE_SERVER) && proto_is_dgram(sock->info.proto)) + { + unsigned int flags = p2mp_iow_flags(m, sock); + + multi_io_process_flags(&m->top, m->multi_io->es, sock, flags); + } + } } tun_set(m->top.c1.tuntap, m->multi_io->es, EVENT_READ, MULTI_IO_TUN, persistent); @@ -457,7 +501,7 @@ multi_io_process_io(struct multi_context *m) } else { - multi_process_io_udp(m, ev_arg->u.sock); + multi_process_io_udp(m, ev_arg->u.sock, e->rwflags); mi = m->pending; } /* monitor and/or handle events that are diff --git a/src/openvpn/multi_io.h b/src/openvpn/multi_io.h index 6b2f59a8d..d6734bd3b 100644 --- a/src/openvpn/multi_io.h +++ b/src/openvpn/multi_io.h @@ -55,7 +55,6 @@ struct multi_io int n_esr; int maxevents; unsigned int tun_rwflags; - unsigned int udp_flags; #ifdef ENABLE_MANAGEMENT unsigned int management_persist_flags; #endif