[prev in list] [next in list] [prev in thread] [next in thread]
List: thttpd
Subject: [THTTPD] 2.25 heap underflow [patch]
From: zell () zell ! best ! vwh ! net
Date: 2003-12-25 7:30:28
[Download RAW message or body]
Hello,
Here's a number of fixes for 2.25, the most serious being an incorrect write that
affects the poll and select implementations (caught by valgrind).
* Original code (poll_del_fd is similar):
--nselect_fds;
select_fds[idx] = select_fds[nselect_fds];
select_fds[nselect_fds] = -1;
select_fdidx[fd] = -1;
select_fdidx[select_fds[idx]] = idx;
If the fd being deleted is the only descriptor being watched, then the above sequence
results in an underflow: select_fdidx[-1] = idx (due to idx and nselect_fds both being
zero).
* Add test for socklen_t to configure, and use socklen_t for socket calls.
* Change description string for /dev/poll to "ioctl", which is the actual system call
used.
* Add a check for HAVE_DEVPOLL before using FD_SETSIZE to limit nfiles.
* Sanity check fd_rw[fd] in fdwatch_check_fd.
* Limit size of kqrevents to nfiles, as only kqevents should ever be greater than
nfiles.
* Replace unnecessary memsets for kqevents and kqrevents with a necessary memset for
kqrfdidx. Otherwise, valgrind/purify will complain of uninitialized reads when
checking for inactive fds in kqueue_check_fd.
* Use nreturned as an additional sanity check in kqueue_check_fd and devpoll_check_fd.
* Simplify /dev/poll implementation to closer resemble kqueue implementation.
* Remove POLLERR from revents bitmask, since POLLERR has already been checked before the
switch statement.
* Restore <sys/time.h> header to mmc.c, to fix Cygwin build.
* Change obvious signal handler variables to volatile. Didn't change hs->cgi_count
or num_connects.
Built under Cygwin/gcc 3.3.1, FreeBSD 4.4/gcc 2.95.3, Linux 2.4/gcc 2.96,
Linux 2.4/icc 8.0, Solaris 2.8/gcc 3.2, and Solaris 2.8/cc 7.0.
diff -Naur thttpd-2.25/configure thttpd-2.25-patch/configure
--- thttpd-2.25/configure 2003-12-19 11:03:32.000000000 -0800
+++ thttpd-2.25-patch/configure 2003-12-24 21:47:01.503539200 -0800
@@ -2366,9 +2366,43 @@
EOF
fi
+echo $ac_n "checking if socklen_t exists""... $ac_c" 1>&6
+echo "configure:2371: checking if socklen_t exists" >&5
+ if eval "test \"`echo '$''{'ac_cv_acme_socklen_t'+set}'`\" = set"; then
+ echo $ac_n "(cached) $ac_c" 1>&6
+else
+ cat > conftest.$ac_ext <<EOF
+#line 2376 "configure"
+#include "confdefs.h"
+
+# include <sys/types.h>
+# include <sys/socket.h>
+int main() {
+socklen_t slen
+; return 0; }
+EOF
+if { (eval echo configure:2385: \"$ac_compile\") 1>&5; (eval $ac_compile) 2>&5; }; then
+ rm -rf conftest*
+ ac_cv_acme_socklen_t=yes
+else
+ echo "configure: failed program was:" >&5
+ cat conftest.$ac_ext >&5
+ rm -rf conftest*
+ ac_cv_acme_socklen_t=no
+fi
+rm -f conftest*
+fi
+
+ echo "$ac_t""$ac_cv_acme_socklen_t" 1>&6
+ if test $ac_cv_acme_socklen_t = yes ; then
+ cat >> confdefs.h <<\EOF
+#define HAVE_SOCKLENT 1
+EOF
+
+ fi
echo $ac_n "checking whether ${MAKE-make} sets \${MAKE}""... $ac_c" 1>&6
-echo "configure:2372: checking whether ${MAKE-make} sets \${MAKE}" >&5
+echo "configure:2406: checking whether ${MAKE-make} sets \${MAKE}" >&5
set dummy ${MAKE-make}; ac_make=`echo "$2" | sed 'y%./+-%__p_%'`
if eval "test \"`echo '$''{'ac_cv_prog_make_${ac_make}_set'+set}'`\" = set"; then
echo $ac_n "(cached) $ac_c" 1>&6
@@ -2406,7 +2440,7 @@
# SVR4 /usr/ucb/install, which tries to use the nonexistent group "staff"
# ./install, which can be erroneously created by make from ./install.sh.
echo $ac_n "checking for a BSD compatible install""... $ac_c" 1>&6
-echo "configure:2410: checking for a BSD compatible install" >&5
+echo "configure:2444: checking for a BSD compatible install" >&5
if test -z "$INSTALL"; then
if eval "test \"`echo '$''{'ac_cv_path_install'+set}'`\" = set"; then
echo $ac_n "(cached) $ac_c" 1>&6
diff -Naur thttpd-2.25/fdwatch.c thttpd-2.25-patch/fdwatch.c
--- thttpd-2.25/fdwatch.c 2003-12-19 11:22:47.000000000 -0800
+++ thttpd-2.25-patch/fdwatch.c 2003-12-24 21:02:42.490065600 -0800
@@ -96,7 +96,7 @@
#else /* HAVE_KQUEUE */
# ifdef HAVE_DEVPOLL
-#define WHICH "devpoll"
+#define WHICH "ioctl"
#define INIT( nfiles ) devpoll_init( nfiles )
#define ADD_FD( fd, rw ) devpoll_add_fd( fd, rw )
#define DEL_FD( fd ) devpoll_del_fd( fd )
@@ -182,10 +182,10 @@
}
#endif /* RLIMIT_NOFILE */
-#if defined(HAVE_SELECT) && ! ( defined(HAVE_POLL) || defined(HAVE_KQUEUE) )
+#if defined(HAVE_SELECT) && ! ( defined(HAVE_POLL) || defined(HAVE_DEVPOLL) || defined(HAVE_KQUEUE) )
/* If we use select(), then we must limit ourselves to FD_SETSIZE. */
nfiles = MIN( nfiles, FD_SETSIZE );
-#endif /* HAVE_SELECT && ! ( HAVE_POLL || HAVE_KQUEUE ) */
+#endif /* HAVE_SELECT && ! ( HAVE_POLL || HAVE_DEVPOLL || HAVE_KQUEUE ) */
/* Initialize the fdwatch data structures. */
nwatches = 0;
@@ -249,7 +249,7 @@
int
fdwatch_check_fd( int fd )
{
- if ( fd < 0 || fd >= nfiles )
+ if ( fd < 0 || fd >= nfiles || fd_rw[fd] == -1 )
{
syslog( LOG_ERR, "bad fd (%d) passed to fdwatch_check_fd!", fd );
return 0;
@@ -302,13 +302,12 @@
return -1;
maxkqevents = nfiles * 2;
kqevents = (struct kevent*) malloc( sizeof(struct kevent) * maxkqevents );
- kqrevents = (struct kevent*) malloc( sizeof(struct kevent) * maxkqevents );
+ kqrevents = (struct kevent*) malloc( sizeof(struct kevent) * nfiles );
kqrfdidx = (int*) malloc( sizeof(int) * nfiles );
if ( kqevents == (struct kevent*) 0 || kqrevents == (struct kevent*) 0 ||
kqrfdidx == (int*) 0 )
return -1;
- (void) memset( kqevents, 0, sizeof(struct kevent) * maxkqevents );
- (void) memset( kqrevents, 0, sizeof(struct kevent) * maxkqevents );
+ (void) memset( kqrfdidx, 0, sizeof(int) * nfiles );
return 0;
}
@@ -359,14 +358,14 @@
if ( timeout_msecs == INFTIM )
r = kevent(
- kq, kqevents, nkqevents, kqrevents, maxkqevents,
+ kq, kqevents, nkqevents, kqrevents, nfiles,
(struct timespec*) 0 );
else
{
struct timespec ts;
ts.tv_sec = timeout_msecs / 1000L;
ts.tv_nsec = ( timeout_msecs % 1000L ) * 1000000L;
- r = kevent( kq, kqevents, nkqevents, kqrevents, maxkqevents, &ts );
+ r = kevent( kq, kqevents, nkqevents, kqrevents, nfiles, &ts );
}
nkqevents = 0;
if ( r == -1 )
@@ -384,11 +383,13 @@
{
int ridx = kqrfdidx[fd];
- if ( ridx < 0 || ridx >= maxkqevents )
+ if ( ridx < 0 || ridx >= nfiles )
{
syslog( LOG_ERR, "bad ridx (%d) in kqueue_check_fd!", ridx );
return 0;
}
+ if ( ridx >= nreturned )
+ return 0;
if ( kqrevents[ridx].ident != fd )
return 0;
if ( kqrevents[ridx].flags & EV_ERROR )
@@ -405,7 +406,7 @@
static int
kqueue_get_fd( int ridx )
{
- if ( ridx < 0 || ridx >= maxkqevents )
+ if ( ridx < 0 || ridx >= nfiles )
{
syslog( LOG_ERR, "bad ridx (%d) in kqueue_get_fd!", ridx );
return -1;
@@ -418,10 +419,10 @@
# ifdef HAVE_DEVPOLL
+static int maxdpevents;
static struct pollfd* dpevents;
static int ndpevents;
static struct pollfd* dprevents;
-static int* dp_fdidx;
static int* dp_rfdidx;
static int dp;
@@ -433,14 +434,13 @@
if ( dp == -1 )
return -1;
(void) fcntl( dp, F_SETFD, 1 );
- dpevents = (struct pollfd*) malloc( sizeof(struct pollfd) * nfiles * 2 );
+ maxdpevents = nfiles * 2;
+ dpevents = (struct pollfd*) malloc( sizeof(struct pollfd) * maxdpevents );
dprevents = (struct pollfd*) malloc( sizeof(struct pollfd) * nfiles );
- dp_fdidx = (int*) malloc( sizeof(int) * nfiles * 2 );
dp_rfdidx = (int*) malloc( sizeof(int) * nfiles );
if ( dpevents == (struct pollfd*) 0 || dprevents == (struct pollfd*) 0 ||
- dp_fdidx == (int*) 0 || dp_rfdidx == (int*) 0 )
+ dp_rfdidx == (int*) 0 )
return -1;
- (void) memset( dp_fdidx, 0, sizeof(int) * nfiles * 2 );
(void) memset( dp_rfdidx, 0, sizeof(int) * nfiles );
return 0;
}
@@ -449,6 +449,11 @@
static void
devpoll_add_fd( int fd, int rw )
{
+ if ( ndpevents >= maxdpevents )
+ {
+ syslog( LOG_ERR, "too many fds in devpoll_add_fd!" );
+ return;
+ }
dpevents[ndpevents].fd = fd;
switch ( rw )
{
@@ -456,68 +461,21 @@
case FDW_WRITE: dpevents[ndpevents].events = POLLOUT; break;
default: break;
}
- dp_fdidx[fd * 2] = ndpevents;
++ndpevents;
}
-static int
-devpoll_compress_fd( int fd, int event )
- {
- int idx = dp_fdidx[fd * 2 + ( event == POLLREMOVE ? 1 : 0 )], r = -1;
-
- if ( idx < 0 || idx >= nfiles * 2 )
- {
- syslog( LOG_ERR, "bad idx (%d) in devpoll_compress_fd!", idx );
- return r;
- }
- if ( idx >= ndpevents )
- return r;
- if ( dpevents[idx].fd != fd )
- return r;
- if ( dpevents[idx].events != event )
- return r;
-
- return idx;
- }
-
-
static void
devpoll_del_fd( int fd )
{
- int idx = devpoll_compress_fd( fd, fd_rw[fd] == FDW_READ ? POLLIN : POLLOUT );
-
- if ( idx == -1 )
- {
- dpevents[ndpevents].fd = fd;
- dpevents[ndpevents].events = POLLREMOVE;
- dp_fdidx[fd * 2 + 1] = ndpevents;
- ++ndpevents;
- }
- else
+ if ( ndpevents >= maxdpevents )
{
- --ndpevents;
- dpevents[idx] = dpevents[ndpevents];
- if ( dpevents[idx].events == POLLREMOVE )
- dp_fdidx[dpevents[idx].fd * 2 + 1] = idx;
- else
- {
- int pos = devpoll_compress_fd( dpevents[idx].fd, POLLREMOVE );
-
- if ( pos < idx )
- dp_fdidx[dpevents[idx].fd * 2] = idx;
- else
- {
- struct pollfd event;
-
- event = dpevents[idx];
- dpevents[idx] = dpevents[pos];
- dpevents[pos] = event;
- dp_fdidx[dpevents[pos].fd * 2] = pos;
- dp_fdidx[dpevents[idx].fd * 2 + 1] = idx;
- }
- }
+ syslog( LOG_ERR, "too many fds in devpoll_del_fd!" );
+ return;
}
+ dpevents[ndpevents].fd = fd;
+ dpevents[ndpevents].events = POLLREMOVE;
+ ++ndpevents;
}
@@ -537,8 +495,8 @@
dvp.dp_timeout = (int) timeout_msecs;
r = ioctl( dp, DP_POLL, &dvp );
- if ( r <= 0 )
- return r;
+ if ( r == -1 )
+ return -1;
for ( i = 0; i < r; ++i )
dp_rfdidx[dprevents[i].fd] = i;
@@ -557,7 +515,8 @@
syslog( LOG_ERR, "bad ridx (%d) in devpoll_check_fd!", ridx );
return 0;
}
- /* Should also check against last value of devpoll_watch(). */
+ if ( ridx >= nreturned )
+ return 0;
if ( dprevents[ridx].fd != fd )
return 0;
if ( dprevents[ridx].revents & POLLERR )
@@ -643,9 +602,9 @@
}
--npoll_fds;
pollfds[idx] = pollfds[npoll_fds];
+ poll_fdidx[pollfds[idx].fd] = idx;
pollfds[npoll_fds].fd = -1;
poll_fdidx[fd] = -1;
- poll_fdidx[pollfds[idx].fd] = idx;
}
@@ -686,8 +645,8 @@
return 0;
switch ( fd_rw[fd] )
{
- case FDW_READ: return pollfds[fdidx].revents & ( POLLIN | POLLERR | POLLHUP | POLLNVAL );
- case FDW_WRITE: return pollfds[fdidx].revents & ( POLLOUT | POLLERR | POLLHUP | POLLNVAL );
+ case FDW_READ: return pollfds[fdidx].revents & ( POLLIN | POLLHUP | POLLNVAL );
+ case FDW_WRITE: return pollfds[fdidx].revents & ( POLLOUT | POLLHUP | POLLNVAL );
default: return 0;
}
}
@@ -778,9 +737,9 @@
--nselect_fds;
select_fds[idx] = select_fds[nselect_fds];
+ select_fdidx[select_fds[idx]] = idx;
select_fds[nselect_fds] = -1;
select_fdidx[fd] = -1;
- select_fdidx[select_fds[idx]] = idx;
FD_CLR( fd, &master_rfdset );
FD_CLR( fd, &master_wfdset );
diff -Naur thttpd-2.25/libhttpd.c thttpd-2.25-patch/libhttpd.c
--- thttpd-2.25/libhttpd.c 2003-12-08 08:21:03.000000000 -0800
+++ thttpd-2.25-patch/libhttpd.c 2003-12-24 21:40:42.959219200 -0800
@@ -102,6 +102,10 @@
typedef long long int64_t;
#endif
+#ifndef HAVE_SOCKLENT
+typedef int socklen_t;
+#endif
+
#ifdef __CYGWIN__
#define timezone _timezone
#endif
@@ -1343,7 +1347,7 @@
vhost_map( httpd_conn* hc )
{
httpd_sockaddr sa;
- int sz;
+ socklen_t sz;
static char* tempfilename;
static size_t maxtempfilename = 0;
char* cp1;
@@ -1654,7 +1658,7 @@
httpd_get_conn( httpd_server* hs, int listen_fd, httpd_conn* hc )
{
httpd_sockaddr sa;
- int sz;
+ socklen_t sz;
if ( ! hc->initialized )
{
diff -Naur thttpd-2.25/mmc.c thttpd-2.25-patch/mmc.c
--- thttpd-2.25/mmc.c 2003-12-09 08:35:06.000000000 -0800
+++ thttpd-2.25-patch/mmc.c 2003-12-24 17:12:59.290798400 -0800
@@ -29,6 +29,7 @@
#include <sys/types.h>
#include <sys/stat.h>
+#include <sys/time.h>
#include <stdlib.h>
#include <unistd.h>
#include <stdio.h>
diff -Naur thttpd-2.25/thttpd.c thttpd-2.25-patch/thttpd.c
--- thttpd-2.25/thttpd.c 2003-12-19 18:42:07.000000000 -0800
+++ thttpd-2.25-patch/thttpd.c 2003-12-24 18:07:13.780524800 -0800
@@ -137,7 +137,7 @@
off_t stats_bytes;
int stats_simultaneous;
-static int got_hup, got_usr1, watchdog_flag;
+static volatile int got_hup, got_usr1, watchdog_flag;
/* Forwards. */
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic