[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[GNUnet-SVN] r37868 - in libmicrohttpd: doc src/microhttpd w32/common
|
From: |
gnunet |
|
Subject: |
[GNUnet-SVN] r37868 - in libmicrohttpd: doc src/microhttpd w32/common |
|
Date: |
Sun, 4 Sep 2016 14:35:46 +0200 |
Author: grothoff
Date: 2016-09-04 14:35:46 +0200 (Sun, 04 Sep 2016)
New Revision: 37868
Modified:
libmicrohttpd/doc/
libmicrohttpd/doc/libmicrohttpd.texi
libmicrohttpd/src/microhttpd/
libmicrohttpd/src/microhttpd/daemon.c
libmicrohttpd/src/microhttpd/internal.h
libmicrohttpd/src/microhttpd/response.c
libmicrohttpd/src/microhttpd/test_upgrade_ssl.c
libmicrohttpd/w32/common/
Log:
-fixing minor issues (leaks, use after free) in recently added upgrade logic
Index: libmicrohttpd/doc
===================================================================
--- libmicrohttpd/doc 2016-09-04 11:25:48 UTC (rev 37867)
+++ libmicrohttpd/doc 2016-09-04 12:35:46 UTC (rev 37868)
Property changes on: libmicrohttpd/doc
___________________________________________________________________
Modified: svn:ignore
## -1,3 +1,5 ##
+manual
+coverage
libmicrohttpd.vr
libmicrohttpd.tps
libmicrohttpd.tp
Modified: libmicrohttpd/doc/libmicrohttpd.texi
===================================================================
--- libmicrohttpd/doc/libmicrohttpd.texi 2016-09-04 11:25:48 UTC (rev
37867)
+++ libmicrohttpd/doc/libmicrohttpd.texi 2016-09-04 12:35:46 UTC (rev
37868)
@@ -161,7 +161,7 @@
@cindex select
MHD supports four basic thread modes and up to three event loop
-styes.
+styles.
The four basic thread modes are external (MHD creates no threads,
event loop is fully managed by the application), internal (MHD creates
Index: libmicrohttpd/src/microhttpd
===================================================================
--- libmicrohttpd/src/microhttpd 2016-09-04 11:25:48 UTC (rev 37867)
+++ libmicrohttpd/src/microhttpd 2016-09-04 12:35:46 UTC (rev 37868)
Property changes on: libmicrohttpd/src/microhttpd
___________________________________________________________________
Modified: svn:ignore
## -1,3 +1,9 ##
+test_upgrade_ssl
+test_upgrade
+test_str_to_value
+test_str_compare
+test_shutdown_select
+test_shutdown_poll
microhttpd_dll_res.rc
test_postprocessor_large
test_postprocessor
Modified: libmicrohttpd/src/microhttpd/daemon.c
===================================================================
--- libmicrohttpd/src/microhttpd/daemon.c 2016-09-04 11:25:48 UTC (rev
37867)
+++ libmicrohttpd/src/microhttpd/daemon.c 2016-09-04 12:35:46 UTC (rev
37868)
@@ -653,7 +653,8 @@
MHD_socket *max_fd,
unsigned int fd_setsize)
{
- if ( (0 == (MHD_EPOLL_STATE_READ_READY & urh->mhd.celi)) &&
+ if ( (urh->out_buffer_off < urh->out_buffer_size) &&
+ (MHD_INVALID_SOCKET != urh->mhd.socket) &&
(! MHD_add_to_fd_set_ (urh->mhd.socket,
rs,
max_fd,
@@ -660,12 +661,14 @@
fd_setsize)) )
return MHD_NO;
if ( (0 == (MHD_EPOLL_STATE_WRITE_READY & urh->mhd.celi)) &&
+ (MHD_INVALID_SOCKET != urh->mhd.socket) &&
(! MHD_add_to_fd_set_ (urh->mhd.socket,
ws,
max_fd,
fd_setsize)) )
return MHD_NO;
- if ( (0 == (MHD_EPOLL_STATE_READ_READY & urh->app.celi)) &&
+ if ( (urh->in_buffer_off < urh->in_buffer_size) &&
+ (MHD_INVALID_SOCKET != urh->connection->socket_fd) &&
(! MHD_add_to_fd_set_ (urh->connection->socket_fd,
rs,
max_fd,
@@ -672,6 +675,7 @@
fd_setsize)) )
return MHD_NO;
if ( (0 == (MHD_EPOLL_STATE_WRITE_READY & urh->app.celi)) &&
+ (MHD_INVALID_SOCKET != urh->connection->socket_fd) &&
(! MHD_add_to_fd_set_ (urh->connection->socket_fd,
ws,
max_fd,
@@ -966,6 +970,12 @@
{
urh->in_buffer_off += res;
}
+ if (0 == res)
+ {
+ /* connection was shut down, signal by shrinking buffer,
+ which will eventually ensure this FD is no longer listed. */
+ urh->in_buffer_size = urh->in_buffer_off;
+ }
}
if ( (0 != (MHD_EPOLL_STATE_WRITE_READY & urh->mhd.celi)) &&
(urh->in_buffer_off > 0) )
@@ -977,8 +987,18 @@
urh->in_buffer_off);
if (-1 == res)
{
- /* FIXME: differenciate by errno? */
- urh->mhd.celi &= ~MHD_EPOLL_STATE_WRITE_READY;
+ int err = MHD_socket_get_error_ ();
+
+ if ( (MHD_SCKT_ERR_IS_EINTR_ (err)) ||
+ (MHD_SCKT_ERR_IS_EAGAIN_ (err)) )
+ urh->mhd.celi &= ~MHD_EPOLL_STATE_WRITE_READY;
+ else
+ {
+ /* persistent / unrecoverable error, treat as
+ if connection was shut down */
+ urh->in_buffer_size = 0;
+ urh->in_buffer_off = 0;
+ }
}
else
{
@@ -1014,6 +1034,12 @@
{
urh->out_buffer_off += res;
}
+ if (0 == res)
+ {
+ /* connection was shut down, signal by shrinking buffer,
+ which will eventually ensure this FD is no longer listed. */
+ urh->out_buffer_size = urh->out_buffer_off;
+ }
fin_read = (0 == res);
}
else
@@ -1045,6 +1071,13 @@
urh->out_buffer_off = 0;
}
}
+ else
+ {
+ /* persistent / unrecoverable error, treat as
+ if connection was shut down */
+ urh->out_buffer_size = 0;
+ urh->out_buffer_off = 0;
+ }
}
if ( (fin_read) &&
(0 == urh->out_buffer_off) &&
@@ -1064,20 +1097,10 @@
thread_main_connection_upgrade (struct MHD_Connection *con)
{
struct MHD_Daemon *daemon = con->daemon;
+ struct MHD_UpgradeResponseHandle *urh = con->urh;
- if (0 == (daemon->options & MHD_USE_SSL))
- {
- /* Here, we need to block until the application
- signals us that it is done with the socket */
- MHD_semaphore_down (con->upgrade_sem);
- MHD_semaphore_destroy (con->upgrade_sem);
- con->upgrade_sem = NULL;
- return;
- }
#if HTTPS_SUPPORT
{
- struct MHD_UpgradeResponseHandle *urh = con->urh;
-
/* Here, we need to bi-directionally forward
until the application tells us that it is done
with the socket; */
@@ -1108,11 +1131,14 @@
#endif
break;
}
- num_ready = MHD_SYS_select_ (max_fd + 1,
- &rs,
- &ws,
- NULL,
- NULL);
+ if (MHD_INVALID_SOCKET != max_fd)
+ num_ready = MHD_SYS_select_ (max_fd + 1,
+ &rs,
+ &ws,
+ NULL,
+ NULL);
+ else
+ num_ready = 0;
if (num_ready < 0)
{
const int err = MHD_socket_get_error_();
@@ -1131,6 +1157,9 @@
&rs,
&ws);
process_urh (urh);
+ if ( (0 == urh->out_buffer_size) &&
+ (0 == urh->in_buffer_size) )
+ break; /* connections died, we have no more purpose here */
}
}
#ifdef HAVE_POLL
@@ -1137,25 +1166,28 @@
else
{
/* use poll() */
- struct pollfd p[2];
const unsigned int timeout = UINT_MAX;
- p[0].fd = urh->connection->socket_fd;
- p[1].fd = urh->mhd.socket;
while (MHD_CONNECTION_UPGRADE == con->state)
{
- if (0 == (MHD_EPOLL_STATE_READ_READY & urh->app.celi))
+ struct pollfd p[2];
+
+ memset (p, 0, sizeof (struct pollfd) * 2);
+ p[0].fd = urh->connection->socket_fd;
+ p[1].fd = urh->mhd.socket;
+ if (urh->in_buffer_off < urh->in_buffer_size)
p[0].events |= POLLIN;
if (0 == (MHD_EPOLL_STATE_WRITE_READY & urh->app.celi))
p[0].events |= POLLOUT;
- if (0 == (MHD_EPOLL_STATE_READ_READY & urh->mhd.celi))
+ if (urh->out_buffer_off < urh->out_buffer_size)
p[1].events |= POLLIN;
if (0 == (MHD_EPOLL_STATE_WRITE_READY & urh->mhd.celi))
p[1].events |= POLLOUT;
- if (MHD_sys_poll_ (p,
- 2,
- timeout) < 0)
+ if ( (0 != (p[0].events | p[1].events)) &&
+ (MHD_sys_poll_ (p,
+ 2,
+ timeout) < 0) )
{
const int err = MHD_socket_get_error_ ();
@@ -1177,6 +1209,9 @@
if (0 != (p[1].revents & POLLOUT))
urh->mhd.celi |= MHD_EPOLL_STATE_WRITE_READY;
process_urh (urh);
+ if ( (0 == urh->out_buffer_size) &&
+ (0 == urh->in_buffer_size) )
+ break; /* connections died, we have no more purpose here */
}
}
/* end POLL */
@@ -1187,6 +1222,13 @@
MHD_PANIC ("This should not be possible\n");
#endif
}
+
+ /* Here, we need to block until the application
+ signals us that it is done with the socket */
+ MHD_semaphore_down (con->upgrade_sem);
+ MHD_semaphore_destroy (con->upgrade_sem);
+ con->upgrade_sem = NULL;
+ free (urh);
}
@@ -2598,6 +2640,7 @@
struct MHD_Connection *next;
#if HTTPS_SUPPORT
struct MHD_UpgradeResponseHandle *urh;
+ struct MHD_UpgradeResponseHandle *urhn;
#endif
unsigned int mask = MHD_USE_SUSPEND_RESUME | MHD_USE_EPOLL_INTERNALLY |
MHD_USE_SELECT_INTERNALLY | MHD_USE_POLL_INTERNALLY |
MHD_USE_THREAD_PER_CONNECTION;
@@ -2650,8 +2693,9 @@
/* handle upgraded HTTPS connections */
#if HTTPS_SUPPORT
- for (urh = daemon->urh_head; NULL != urh; urh = urh->next)
+ for (urh = daemon->urh_head; NULL != urh; urh = urhn)
{
+ urhn = urh->next;
/* update urh state based on select() output */
urh_from_fdset (urh,
read_fd_set,
@@ -2934,13 +2978,13 @@
for (urh = daemon->urh_head; NULL != urh; urh = urh->next)
{
p[poll_server+i].fd = urh->connection->socket_fd;
- if (0 == (MHD_EPOLL_STATE_READ_READY & urh->app.celi))
+ if (urh->in_buffer_off < urh->in_buffer_size)
p[poll_server+i].events |= POLLIN;
if (0 == (MHD_EPOLL_STATE_WRITE_READY & urh->app.celi))
p[poll_server+i].events |= POLLOUT;
i++;
p[poll_server+i].fd = urh->mhd.socket;
- if (0 == (MHD_EPOLL_STATE_READ_READY & urh->mhd.celi))
+ if (urh->out_buffer_off < urh->out_buffer_size)
p[poll_server+i].events |= POLLIN;
if (0 == (MHD_EPOLL_STATE_WRITE_READY & urh->mhd.celi))
p[poll_server+i].events |= POLLOUT;
@@ -2952,7 +2996,9 @@
free(p);
return MHD_YES;
}
- if (MHD_sys_poll_(p, poll_server + num_connections, timeout) < 0)
+ if (MHD_sys_poll_(p,
+ poll_server + num_connections,
+ timeout) < 0)
{
const int err = MHD_socket_get_error_ ();
if (MHD_SCKT_ERR_IS_EINTR_ (err))
@@ -2988,7 +3034,7 @@
next = pos->next;
/* first, sanity checks */
if (i >= num_connections)
- continue; /* connection list changed somehow, retry later ... */
+ break; /* connection list changed somehow, retry later ... */
if (p[poll_server+i].fd != pos->socket_fd)
continue; /* fd mismatch, something else happened, retry later ... */
call_handlers (pos,
@@ -2995,10 +3041,13 @@
0 != (p[poll_server+i].revents & POLLIN),
0 != (p[poll_server+i].revents & POLLOUT),
MHD_NO);
+ i++;
}
#if HTTPS_SUPPORT
for (urh = daemon->urh_head; NULL != urh; urh = urhn)
{
+ if (i >= num_connections)
+ break; /* connection list changed somehow, retry later ... */
urhn = urh->next;
if (p[poll_server+i].fd != urh->connection->socket_fd)
continue; /* fd mismatch, something else happened, retry later ... */
@@ -3148,6 +3197,7 @@
struct epoll_event events[MAX_EVENTS];
int num_events;
unsigned int i;
+ unsigned int j;
num_events = MAX_EVENTS;
while (MAX_EVENTS == num_events)
@@ -3172,8 +3222,35 @@
for (i=0;i<(unsigned int) num_events;i++)
{
struct UpgradeEpollHandle *ueh = events[i].data.ptr;
- struct MHD_UpgradeResponseHandle *urh = ueh->urh;
+ struct MHD_UpgradeResponseHandle *urh;
+ if (NULL == ueh)
+ continue; /* was killed, see below */
+ urh = ueh->urh;
+
+ /* In case we get two events for the same upgrade handle,
+ squash them together (otherwise the first one may
+ cause us to free the 'urh', and the second one then
+ causes a use-after-free). */
+ for (j=i+1;j< (unsigned int) num_events;j++)
+ {
+ struct UpgradeEpollHandle *uehj = events[j].data.ptr;
+ struct MHD_UpgradeResponseHandle *urhj;
+
+ if (NULL == uehj)
+ continue; /* was killed, see below */
+ urhj = uehj->urh;
+
+ if (urh == urhj) /* yep, indeed the same! */
+ {
+ if (0 != (events[j].events & EPOLLIN))
+ uehj->celi |= MHD_EPOLL_STATE_READ_READY;
+ if (0 != (events[j].events & EPOLLOUT))
+ uehj->celi |= MHD_EPOLL_STATE_WRITE_READY;
+ }
+ events[j].data.ptr = NULL; /* kill this one */
+ }
+
/* Update our state based on what is ready according to epoll() */
if (0 != (events[i].events & EPOLLIN))
ueh->celi |= MHD_EPOLL_STATE_READ_READY;
Modified: libmicrohttpd/src/microhttpd/internal.h
===================================================================
--- libmicrohttpd/src/microhttpd/internal.h 2016-09-04 11:25:48 UTC (rev
37867)
+++ libmicrohttpd/src/microhttpd/internal.h 2016-09-04 12:35:46 UTC (rev
37868)
@@ -859,7 +859,6 @@
*/
TransmitCallback send_cls;
-#if HTTPS_SUPPORT
/**
* If this connection was upgraded and if we are using
* #MHD_USE_THREAD_PER_CONNECTION, this points to the
@@ -869,6 +868,7 @@
*/
struct MHD_UpgradeResponseHandle *urh;
+#if HTTPS_SUPPORT
/**
* If this connection was upgraded and if we are using
* #MHD_USE_THREAD_PER_CONNECTION without encryption,
Modified: libmicrohttpd/src/microhttpd/response.c
===================================================================
--- libmicrohttpd/src/microhttpd/response.c 2016-09-04 11:25:48 UTC (rev
37867)
+++ libmicrohttpd/src/microhttpd/response.c 2016-09-04 12:35:46 UTC (rev
37868)
@@ -627,19 +627,16 @@
/* Application is done with this connection, tear it down! */
if (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION) )
{
- if (0 == (daemon->options & MHD_USE_SSL) )
- {
- /* just need to signal the thread that we are done */
- MHD_semaphore_up (connection->upgrade_sem);
- }
#if HTTPS_SUPPORT
- else
+ if (0 != (daemon->options & MHD_USE_SSL) )
{
- /* signal thread by shutdown() of 'app' socket */
+ /* signal thread that app is done by shutdown() of 'app' socket */
shutdown (urh->app.socket,
SHUT_RDWR);
}
#endif
+ /* need to signal the thread that we are done */
+ MHD_semaphore_up (connection->upgrade_sem);
return MHD_YES;
}
#if HTTPS_SUPPORT
@@ -755,6 +752,12 @@
urh->mhd.celi = MHD_EPOLL_STATE_UNREADY;
pool = connection->pool;
avail = MHD_pool_get_free (pool);
+ if (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION) )
+ {
+ /* Need to give the thread something to block on... */
+ connection->upgrade_sem = MHD_semaphore_create (0);
+ }
+
if (avail < 8)
{
/* connection's pool is totally at the limit,
@@ -871,7 +874,8 @@
{
/* Need to give the thread something to block on... */
connection->upgrade_sem = MHD_semaphore_create (0);
- if (NULL == connection->upgrade_sem)
+ connection->urh = urh;
+ if (NULL == connection->upgrade_sem)
{
#ifdef HAVE_MESSAGES
MHD_DLOG (daemon,
Modified: libmicrohttpd/src/microhttpd/test_upgrade_ssl.c
===================================================================
--- libmicrohttpd/src/microhttpd/test_upgrade_ssl.c 2016-09-04 11:25:48 UTC
(rev 37867)
+++ libmicrohttpd/src/microhttpd/test_upgrade_ssl.c 2016-09-04 12:35:46 UTC
(rev 37868)
@@ -400,7 +400,9 @@
MHD_socket sock;
pid_t pid;
- d = MHD_start_daemon (flags | MHD_USE_DEBUG | MHD_USE_SUSPEND_RESUME |
MHD_USE_TLS,
+ if (0 == (flags & MHD_USE_THREAD_PER_CONNECTION))
+ flags |= MHD_USE_SUSPEND_RESUME;
+ d = MHD_start_daemon (flags | MHD_USE_DEBUG | MHD_USE_TLS,
1080,
NULL, NULL,
&ahc_upgrade, NULL,
@@ -442,13 +444,21 @@
if (0 != system ("openssl version 1> /dev/null"))
return 77; /* openssl not available, can't run the test */
+
+ /* Test thread-per-connection */
+ error_count += test_upgrade_internal (MHD_USE_THREAD_PER_CONNECTION,
+ 0);
+ error_count += test_upgrade_internal (MHD_USE_THREAD_PER_CONNECTION |
MHD_USE_POLL,
+ 0);
+
+ /* Test different event loops, with and without thread pool */
error_count += test_upgrade_internal (MHD_USE_SELECT_INTERNALLY,
- 1);
+ 0);
error_count += test_upgrade_internal (MHD_USE_SELECT_INTERNALLY,
2);
#ifdef HAVE_POLL
error_count += test_upgrade_internal (MHD_USE_POLL_INTERNALLY,
- 1);
+ 0);
error_count += test_upgrade_internal (MHD_USE_POLL_INTERNALLY,
2);
#endif
@@ -455,12 +465,14 @@
#ifdef EPOLL_SUPPORT
error_count += test_upgrade_internal (MHD_USE_EPOLL_INTERNALLY |
MHD_USE_TLS_EPOLL_UPGRADE,
- 1);
+ 0);
error_count += test_upgrade_internal (MHD_USE_EPOLL_INTERNALLY |
MHD_USE_TLS_EPOLL_UPGRADE,
2);
#endif
- if (error_count != 0)
+
+ /* report result */
+ if (0 != error_count)
fprintf (stderr,
"Error (code: %u)\n",
error_count);
Index: libmicrohttpd/w32/common
===================================================================
--- libmicrohttpd/w32/common 2016-09-04 11:25:48 UTC (rev 37867)
+++ libmicrohttpd/w32/common 2016-09-04 12:35:46 UTC (rev 37868)
Property changes on: libmicrohttpd/w32/common
___________________________________________________________________
Added: svn:ignore
## -0,0 +1 ##
+microhttpd_dll_res_vc.rc
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- [GNUnet-SVN] r37868 - in libmicrohttpd: doc src/microhttpd w32/common,
gnunet <=