[libmicrohttpd] branch master updated: Upgraded connection: fixed use-after-free for thread-per-connection

[gnunet]

This is an automated email from the git hooks/post-receive script.

karlson2k pushed a commit to branch master
in repository libmicrohttpd.

The following commit(s) were added to refs/heads/master by this push:
     new c5b4a9de Upgraded connection: fixed use-after-free for 
thread-per-connection
c5b4a9de is described below

commit c5b4a9deb550ab5a8a0bb36103a9547da21f8982
Author: Evgeny Grin (Karlson2k) <k2k@narod.ru>
AuthorDate: Fri Oct 30 19:09:59 2020 +0300

    Upgraded connection: fixed use-after-free for thread-per-connection
---
 src/microhttpd/connection.c |  2 +-
 src/microhttpd/daemon.c     | 28 +++++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/microhttpd/connection.c b/src/microhttpd/connection.c
index 092c729d..b85597c4 100644
--- a/src/microhttpd/connection.c
+++ b/src/microhttpd/connection.c
@@ -728,7 +728,7 @@ MHD_connection_finish_forward_ (struct MHD_Connection 
*connection)
   }
   /* Socketpair sockets will remain open as they will be
    * used with MHD_UPGRADE_ACTION_CLOSE. They will be
-   * closed by MHD_cleanup_upgraded_connection_() during
+   * closed by cleanup_upgraded_connection() during
    * connection's final cleanup.
    */
 }
diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
index f2e049eb..640964b8 100644
--- a/src/microhttpd/daemon.c
+++ b/src/microhttpd/daemon.c
@@ -3172,7 +3172,8 @@ resume_suspended_connections (struct MHD_Daemon *daemon)
   {
     prev = daemon->suspended_connections_tail;
     /* During shutdown check for resuming is forced. */
-    mhd_assert ((NULL != prev) || (daemon->shutdown));
+    mhd_assert ((NULL != prev) || (daemon->shutdown) || \
+                (0 != (daemon->options & MHD_ALLOW_UPGRADE)));
   }
 
   daemon->resuming = false;
@@ -7024,6 +7025,7 @@ close_all_connections (struct MHD_Daemon *daemon)
   /* 'daemon->urh_head' is not used in thread-per-connection mode. */
   for (urh = daemon->urh_tail; NULL != urh; urh = urhn)
   {
+    mhd_assert (! used_thr_p_c);
     urhn = urh->prev;
     /* call generic forwarding function for passing data
        with chance to detect that application is done. */
@@ -7093,6 +7095,29 @@ close_all_connections (struct MHD_Daemon *daemon)
   if (NULL != daemon->suspended_connections_head)
     MHD_PANIC (_ (
                  "MHD_stop_daemon() called while we have suspended 
connections.\n"));
+#if defined(UPGRADE_SUPPORT) && defined(HTTPS_SUPPORT)
+  if (upg_allowed && used_tls && used_thr_p_c)
+  {
+    /* "Upgraded" threads may be running in parallel. Connection will not be
+     * moved to the "cleanup list" until connection's thread finishes.
+     * We must ensure that all "upgraded" connections are finished otherwise
+     * connection may stay in "suspended" list and will not be cleaned. */
+    for (pos = daemon->suspended_connections_tail; NULL != pos; pos = 
pos->prev)
+    {
+      /* Any connection found here is "upgraded" connection, normal suspended
+       * connections are already removed from this list. */
+      mhd_assert (NULL != pos->urh);
+      if (! pos->thread_joined)
+      {
+        /* No need to unlock "cleanup" mutex as upgraded connection
+         * doesn't manipulate "cleanup" list. */
+        if (! MHD_join_thread_ (pos->pid.handle))
+          MHD_PANIC (_ ("Failed to join a thread.\n"));
+        pos->thread_joined = true;
+      }
+    }
+  }
+#endif
   for (pos = daemon->connections_tail; NULL != pos; pos = pos->prev)
   {
     shutdown (pos->socket_fd,
@@ -7143,6 +7168,7 @@ close_all_connections (struct MHD_Daemon *daemon)
   }
 #endif /* UPGRADE_SUPPORT */
 
+  mhd_assert (NULL == daemon->suspended_connections_head);
   /* now that we're alone, move everyone to cleanup */
   while (NULL != (pos = daemon->connections_tail))
   {

-- 
To stop receiving notification emails like this one, please contact
gnunet@gnunet.org.