From: Gianmarco De Gregori <[email protected]> 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 <[email protected]> Acked-by: Antonio Quartulli <[email protected]> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1635 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1635 This mail reflects revision 5 of this Change. Acked-by according to Gerrit (reflected above): Antonio Quartulli <[email protected]> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index d24f534..f2dc2da 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -2030,9 +2030,8 @@ 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,39 +2122,8 @@ * (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); } /* @@ -2165,8 +2133,6 @@ 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 +2151,7 @@ */ 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 0d3e492..d4edfba 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -68,7 +68,7 @@ extern counter_type link_write_bytes_global; -void get_io_flags_udp(struct context *c, struct multi_io *multi_io, const unsigned int flags); +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 432c79a..bb89221 100644 --- a/src/openvpn/mudp.c +++ b/src/openvpn/mudp.c @@ -336,18 +336,17 @@ * 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)) @@ -355,25 +354,25 @@ multi_process_incoming_link(m, NULL, mpp_flags, sock); } } - - m->multi_io->udp_flags = ES_ERROR; } /* - * Return the io_wait() flags appropriate for + * Return the io flags appropriate for * a point-to-multipoint tunnel. */ unsigned int -p2mp_iow_flags(const struct multi_context *m) +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 (LINK_OUT(&m->pending->context)) + + if (LINK_OUT(&m->pending->context) && m->pending->context.c2.link_sockets[0] == sock) { flags |= IOW_TO_LINK; } diff --git a/src/openvpn/mudp.h b/src/openvpn/mudp.h index 005ee10..dfc69b9 100644 --- a/src/openvpn/mudp.h +++ b/src/openvpn/mudp.h @@ -30,9 +30,9 @@ struct context; struct multi_context; -unsigned int p2mp_iow_flags(const struct multi_context *m); +unsigned int p2mp_iow_flags(const struct multi_context *m, struct link_socket *sock); -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 e6f4e9c..1334a74 100644 --- a/src/openvpn/multi_io.c +++ b/src/openvpn/multi_io.c @@ -171,7 +171,17 @@ 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 +467,7 @@ } 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 6b2f59a..d6734bd 100644 --- a/src/openvpn/multi_io.h +++ b/src/openvpn/multi_io.h @@ -55,7 +55,6 @@ int n_esr; int maxevents; unsigned int tun_rwflags; - unsigned int udp_flags; #ifdef ENABLE_MANAGEMENT unsigned int management_persist_flags; #endif _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
