Author: cgutman Date: Sun Sep 20 17:00:08 2009 New Revision: 43095
URL: http://svn.reactos.org/svn/reactos?rev=43095&view=rev Log: - Partial rewrite of dispatch() - Header cleanup - Fix several bugs related to null pointer access and out-of-bounds array access - Found by Amine Khaldi - This should fix the issue with dhcp crashing and hogging tons of cpu time
Modified: trunk/reactos/base/services/dhcp/adapter.c trunk/reactos/base/services/dhcp/dhclient.c trunk/reactos/base/services/dhcp/dispatch.c trunk/reactos/base/services/dhcp/include/dhcpd.h trunk/reactos/base/services/dhcp/include/rosdhcp.h trunk/reactos/base/services/dhcp/options.c
Modified: trunk/reactos/base/services/dhcp/adapter.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/dhcp/adapter.... ============================================================================== --- trunk/reactos/base/services/dhcp/adapter.c [iso-8859-1] (original) +++ trunk/reactos/base/services/dhcp/adapter.c [iso-8859-1] Sun Sep 20 17:00:08 2009 @@ -169,10 +169,9 @@ Adapter->DhclientState.state = S_STATIC;
Netmask = RegReadString( AdapterKey, NULL, "Subnetmask" ); - if( !Netmask ) Netmask = "255.255.255.0";
Status = AddIPAddress( inet_addr( IPAddress ), - inet_addr( Netmask ), + inet_addr( Netmask ? Netmask : "255.255.255.0" ), Adapter->IfMib.dwIndex, &Adapter->NteContext, &Adapter->NteInstance ); @@ -257,7 +256,7 @@ DH_DbgPrint(MID_TRACE,("Getting adapter %d attributes\n", Table->table[i].dwIndex));
- if (AdapterFindByHardwareAddress(Table->table[i].bPhysAddr, Table->table[i].dwPhysAddrLen)) + if ((Adapter = AdapterFindByHardwareAddress(Table->table[i].bPhysAddr, Table->table[i].dwPhysAddrLen))) { /* This is an existing adapter */ if (InterfaceConnected(Table->table[i])) {
Modified: trunk/reactos/base/services/dhcp/dhclient.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/dhcp/dhclient... ============================================================================== --- trunk/reactos/base/services/dhcp/dhclient.c [iso-8859-1] (original) +++ trunk/reactos/base/services/dhcp/dhclient.c [iso-8859-1] Sun Sep 20 17:00:08 2009 @@ -1576,8 +1576,10 @@
if (!leaseFile) { /* XXX */ leaseFile = fopen(path_dhclient_db, "w"); - if (!leaseFile) + if (!leaseFile) { error("can't create %s", path_dhclient_db); + return; + } }
fprintf(leaseFile, "lease {\n"); @@ -1600,17 +1602,20 @@ lease->options[i].len, 1, 1));
t = gmtime(&lease->renewal); - fprintf(leaseFile, " renew %d %d/%d/%d %02d:%02d:%02d;\n", - t->tm_wday, t->tm_year + 1900, t->tm_mon + 1, t->tm_mday, - t->tm_hour, t->tm_min, t->tm_sec); + if (t) + fprintf(leaseFile, " renew %d %d/%d/%d %02d:%02d:%02d;\n", + t->tm_wday, t->tm_year + 1900, t->tm_mon + 1, t->tm_mday, + t->tm_hour, t->tm_min, t->tm_sec); t = gmtime(&lease->rebind); - fprintf(leaseFile, " rebind %d %d/%d/%d %02d:%02d:%02d;\n", - t->tm_wday, t->tm_year + 1900, t->tm_mon + 1, t->tm_mday, - t->tm_hour, t->tm_min, t->tm_sec); + if (t) + fprintf(leaseFile, " rebind %d %d/%d/%d %02d:%02d:%02d;\n", + t->tm_wday, t->tm_year + 1900, t->tm_mon + 1, t->tm_mday, + t->tm_hour, t->tm_min, t->tm_sec); t = gmtime(&lease->expiry); - fprintf(leaseFile, " expire %d %d/%d/%d %02d:%02d:%02d;\n", - t->tm_wday, t->tm_year + 1900, t->tm_mon + 1, t->tm_mday, - t->tm_hour, t->tm_min, t->tm_sec); + if (t) + fprintf(leaseFile, " expire %d %d/%d/%d %02d:%02d:%02d;\n", + t->tm_wday, t->tm_year + 1900, t->tm_mon + 1, t->tm_mday, + t->tm_hour, t->tm_min, t->tm_sec); fprintf(leaseFile, "}\n"); fflush(leaseFile); } @@ -1765,7 +1770,7 @@ config->defaults[i].data, ip->client-> config->defaults[i].len); - dp[len] = '\0'; + dp[len-1] = '\0'; } } else { dp = ip->client->
Modified: trunk/reactos/base/services/dhcp/dispatch.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/dhcp/dispatch... ============================================================================== --- trunk/reactos/base/services/dhcp/dispatch.c [iso-8859-1] (original) +++ trunk/reactos/base/services/dhcp/dispatch.c [iso-8859-1] Sun Sep 20 17:00:08 2009 @@ -63,11 +63,16 @@ void dispatch(void) { - int count, i, to_msec, nfds, err; + int count, to_msec, err; struct protocol *l; fd_set fds; time_t howlong, cur_time; struct timeval timeval; + + if (!AdapterDiscover()) { + AdapterStop(); + return; + }
ApiLock();
@@ -76,17 +81,6 @@ * Call any expired timeouts, and then if there's still * a timeout registered, time out the select call then. */ - another: - if (!AdapterDiscover()) { - AdapterStop(); - break; - } - - for (l = protocols, nfds = 0; l; l = l->next) - nfds++; - - FD_ZERO(&fds); - time(&cur_time);
if (timeouts) { @@ -98,7 +92,7 @@ (*(t->func))(t->what); t->next = free_timeouts; free_timeouts = t; - goto another; + continue; }
/* @@ -112,49 +106,30 @@ howlong = INT_MAX / 1000; to_msec = howlong * 1000; } else - to_msec = -1; + to_msec = 5000;
/* Set up the descriptors to be polled. */ - for (i = 0, l = protocols; l; l = l->next) { - struct interface_info *ip = l->local; - - if (ip && (l->handler != got_one || !ip->dead)) { - FD_SET(l->fd, &fds); - i++; - } - } - - if (i == 0) { - /* Wait for 5 seconds before looking for more interfaces */ - Sleep(5000); - continue; - } else { - /* Wait for a packet or a timeout... XXX */ - timeval.tv_sec = to_msec / 1000; - timeval.tv_usec = (to_msec % 1000) * 1000; - } + FD_ZERO(&fds); + + for (l = protocols; l; l = l->next) + FD_SET(l->fd, &fds); + + /* Wait for a packet or a timeout... XXX */ + timeval.tv_sec = to_msec / 1000; + timeval.tv_usec = to_msec % 1000;
ApiUnlock();
- count = select(nfds, &fds, NULL, NULL, &timeval); + if (protocols) + count = select(0, &fds, NULL, NULL, &timeval); + else { + Sleep(to_msec); + count = 0; + } + + ApiLock();
DH_DbgPrint(MID_TRACE,("Select: %d\n", count)); - - /* Review poll output */ - for (i = 0, l = protocols; l; l = l->next) { - struct interface_info *ip = l->local; - - if (ip && (l->handler != got_one || !ip->dead)) { - DH_DbgPrint - (MID_TRACE, - ("set(%d) -> %s\n", - l->fd, FD_ISSET(l->fd, &fds) ? "true" : "false")); - i++; - } - } - - - ApiLock();
/* Not likely to be transitory... */ if (count == SOCKET_ERROR) { @@ -163,7 +138,6 @@ break; }
- i = 0; for (l = protocols; l; l = l->next) { struct interface_info *ip; ip = l->local; @@ -173,7 +147,6 @@ DH_DbgPrint(MID_TRACE,("Handling %x\n", l)); (*(l->handler))(l); } - i++; } } } while (1); @@ -269,8 +242,10 @@ q->what = what; } else { q = malloc(sizeof(struct timeout)); - if (!q) + if (!q) { error("Can't allocate timeout structure!"); + return; + } q->func = where; q->what = what; }
Modified: trunk/reactos/base/services/dhcp/include/dhcpd.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/dhcp/include/... ============================================================================== --- trunk/reactos/base/services/dhcp/include/dhcpd.h [iso-8859-1] (original) +++ trunk/reactos/base/services/dhcp/include/dhcpd.h [iso-8859-1] Sun Sep 20 17:00:08 2009 @@ -101,7 +101,6 @@ #include <fcntl.h> #include <limits.h> //#include <unistd.h> -#include <stdarg.h> #include <stdio.h> #include <stdlib.h> #include <string.h>
Modified: trunk/reactos/base/services/dhcp/include/rosdhcp.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/dhcp/include/... ============================================================================== --- trunk/reactos/base/services/dhcp/include/rosdhcp.h [iso-8859-1] (original) +++ trunk/reactos/base/services/dhcp/include/rosdhcp.h [iso-8859-1] Sun Sep 20 17:00:08 2009 @@ -10,7 +10,6 @@ #include <dhcpcsdk.h> #include <stdio.h> #include <io.h> -#include <setjmp.h> #include "stdint.h" #include "predec.h" #include <dhcp/rosdhcp_public.h>
Modified: trunk/reactos/base/services/dhcp/options.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/dhcp/options.... ============================================================================== --- trunk/reactos/base/services/dhcp/options.c [iso-8859-1] (original) +++ trunk/reactos/base/services/dhcp/options.c [iso-8859-1] Sun Sep 20 17:00:08 2009 @@ -175,9 +175,11 @@ * for clients, but what the heck... */ t = calloc(1, len + packet->options[code].len + 1); - if (!t) + if (!t) { error("Can't expand storage for option %s.", dhcp_options[code].name); + return; + } memcpy(t, packet->options[code].data, packet->options[code].len); memcpy(t + packet->options[code].len,