[PATCH v5] Multisocket: use event engine rwflags for UDP I/O

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