From: Danny Mayer Date: Thu, 24 May 2007 12:08:34 +0000 (-0400) Subject: Bug #527 Don't write from source address length to wrong location X-Git-Tag: NTP_4_2_4P2_RC3~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d927a54b41d28beb29dc87664183550b443e2707;p=thirdparty%2Fntp.git Bug #527 Don't write from source address length to wrong location bk: 465580425FOZCIMWFxQqymm_KiGBAg --- diff --git a/include/recvbuff.h b/include/recvbuff.h index 1df7b371d..5feadc975 100644 --- a/include/recvbuff.h +++ b/include/recvbuff.h @@ -66,6 +66,7 @@ struct recvbuf { #else struct sockaddr_storage srcadr; /* where packet came from */ #endif + int src_addr_len; /* source address length */ struct interface *dstadr; /* interface datagram arrived thru */ SOCKET fd; /* fd on which it was received */ int msg_flags; /* Flags received about the packet */ @@ -76,6 +77,7 @@ struct recvbuf { struct pkt X_recv_pkt; u_char X_recv_buffer[RX_BUFF_SIZE]; } recv_space; + int used; #define recv_pkt recv_space.X_recv_pkt #define recv_buffer recv_space.X_recv_buffer }; diff --git a/libntp/recvbuff.c b/libntp/recvbuff.c index 7bee5a613..39657daf0 100644 --- a/libntp/recvbuff.c +++ b/libntp/recvbuff.c @@ -21,8 +21,8 @@ static u_long volatile total_recvbufs; /* total recvbufs currently in use */ static u_long volatile lowater_adds; /* number of times we have added memory */ -static ISC_LIST(recvbuf_t) full_list; /* Currently used recv buffers */ -static ISC_LIST(recvbuf_t) free_list; /* Currently unused buffers */ +static ISC_LIST(recvbuf_t) full_recv_list; /* Currently used recv buffers */ +static ISC_LIST(recvbuf_t) free_recv_list; /* Currently unused buffers */ #if defined(SYS_WINNT) @@ -88,7 +88,8 @@ create_buffers(int nbufs) return (0); for (i = 0; i < nbufs; i++) { - ISC_LIST_APPEND(free_list, bufp, link); + memset((char *) bufp, 0, sizeof(recvbuf_t)); + ISC_LIST_APPEND(free_recv_list, bufp, link); bufp++; free_recvbufs++; total_recvbufs++; @@ -104,8 +105,8 @@ init_recvbuff(int nbufs) /* * Init buffer free list and stat counters */ - ISC_LIST_INIT(full_list); - ISC_LIST_INIT(free_list); + ISC_LIST_INIT(full_recv_list); + ISC_LIST_INIT(free_recv_list); free_recvbufs = total_recvbufs = 0; full_recvbufs = lowater_adds = 0; @@ -123,8 +124,16 @@ init_recvbuff(int nbufs) void freerecvbuf(recvbuf_t *rb) { + if (rb == NULL) { + msyslog(LOG_ERR, "freerecvbuff received NULL buffer"); + return; + } + LOCK(); - ISC_LIST_APPEND(free_list, rb, link); + (rb->used)--; + if (rb->used != 0) + msyslog(LOG_ERR, "******** freerecvbuff non-zero usage: %d *******", rb->used); + ISC_LIST_APPEND(free_recv_list, rb, link); #if defined SYS_WINNT rb->wsabuff.len = RX_BUFF_SIZE; rb->wsabuff.buf = (char *) rb->recv_buffer; @@ -137,8 +146,12 @@ freerecvbuf(recvbuf_t *rb) void add_full_recv_buffer(recvbuf_t *rb) { + if (rb == NULL) { + msyslog(LOG_ERR, "add_full_recv_buffer received NULL buffer"); + return; + } LOCK(); - ISC_LIST_APPEND(full_list, rb, link); + ISC_LIST_APPEND(full_recv_list, rb, link); full_recvbufs++; UNLOCK(); } @@ -148,7 +161,7 @@ get_free_recv_buffer(void) { recvbuf_t * buffer = NULL; LOCK(); - buffer = ISC_LIST_HEAD(free_list); + buffer = ISC_LIST_HEAD(free_recv_list); if (buffer == NULL) { /* @@ -160,7 +173,7 @@ get_free_recv_buffer(void) UNLOCK(); return (NULL); } - buffer = ISC_LIST_HEAD(free_list); + buffer = ISC_LIST_HEAD(free_recv_list); if (buffer == NULL) { msyslog(LOG_ERR, "Failed to obtain more memory for recvbufs"); @@ -168,9 +181,10 @@ get_free_recv_buffer(void) return (NULL); } } - ISC_LIST_DEQUEUE(free_list, buffer, link); + ISC_LIST_DEQUEUE(free_recv_list, buffer, link); free_recvbufs--; initialise_buffer(buffer); + (buffer->used)++; UNLOCK(); return (buffer); } @@ -180,10 +194,10 @@ get_full_recv_buffer(void) { recvbuf_t *rbuf; LOCK(); - rbuf = ISC_LIST_HEAD(full_list); + rbuf = ISC_LIST_HEAD(full_recv_list); if (rbuf != NULL) { - ISC_LIST_DEQUEUE(full_list, rbuf, link); + ISC_LIST_DEQUEUE(full_recv_list, rbuf, link); --full_recvbufs; } else @@ -202,7 +216,7 @@ get_full_recv_buffer(void) */ isc_boolean_t has_full_recv_buffer(void) { - if (ISC_LIST_HEAD(full_list) != NULL) + if (ISC_LIST_HEAD(full_recv_list) != NULL) return (ISC_TRUE); else return (ISC_FALSE); diff --git a/ports/winnt/libntp/transmitbuff.c b/ports/winnt/libntp/transmitbuff.c index 8a0789a6d..5d2679cc9 100644 --- a/ports/winnt/libntp/transmitbuff.c +++ b/ports/winnt/libntp/transmitbuff.c @@ -25,8 +25,8 @@ static volatile u_long full_transmitbufs = 0; /* number of transmitbufs on fulllist */ static volatile u_long free_transmitbufs = 0; /* number of transmitbufs on freelist */ -ISC_LIST(transmitbuf_t) free_list; /* Currently used transmit buffers */ -ISC_LIST(transmitbuf_t) full_list; /* Currently used transmit buffers */ +ISC_LIST(transmitbuf_t) free_transmit_list; /* Currently used transmit buffers */ +ISC_LIST(transmitbuf_t) full_transmit_list; /* Currently used transmit buffers */ static u_long total_transmitbufs = 0; /* total transmitbufs currently in use */ static u_long lowater_additions = 0; /* number of times we have added memory */ @@ -47,7 +47,7 @@ initialise_buffer(transmitbuf *buff) static void add_buffer_to_freelist(transmitbuf *tb) { - ISC_LIST_APPEND(free_list, tb, link); + ISC_LIST_APPEND(free_transmit_list, tb, link); free_transmitbufs++; } @@ -75,8 +75,8 @@ init_transmitbuff(void) /* * Init buffer free list and stat counters */ - ISC_LIST_INIT(full_list); - ISC_LIST_INIT(free_list); + ISC_LIST_INIT(full_transmit_list); + ISC_LIST_INIT(free_transmit_list); free_transmitbufs = total_transmitbufs = 0; full_transmitbufs = lowater_additions = 0; create_buffers(TRANSMIT_INIT); @@ -88,12 +88,12 @@ static void delete_buffer_from_full_list(transmitbuf_t *tb) { transmitbuf_t *next = NULL; - transmitbuf_t *lbuf = ISC_LIST_HEAD(full_list); + transmitbuf_t *lbuf = ISC_LIST_HEAD(full_transmit_list); while (lbuf != NULL) { next = ISC_LIST_NEXT(lbuf, link); if (lbuf == tb) { - ISC_LIST_DEQUEUE(full_list, lbuf, link); + ISC_LIST_DEQUEUE(full_transmit_list, lbuf, link); break; } else @@ -121,12 +121,12 @@ get_free_transmit_buffer(void) if (free_transmitbufs <= 0) { create_buffers(TRANSMIT_INC); } - buffer = ISC_LIST_HEAD(free_list); + buffer = ISC_LIST_HEAD(free_transmit_list); if (buffer != NULL) { - ISC_LIST_DEQUEUE(free_list, buffer, link); + ISC_LIST_DEQUEUE(free_transmit_list, buffer, link); free_transmitbufs--; - ISC_LIST_APPEND(full_list, buffer, link); + ISC_LIST_APPEND(full_transmit_list, buffer, link); full_transmitbufs++; } UNLOCK(&TransmitLock); diff --git a/ports/winnt/ntpd/ntp_iocompletionport.c b/ports/winnt/ntpd/ntp_iocompletionport.c index b89a36227..39ae4f15a 100644 --- a/ports/winnt/ntpd/ntp_iocompletionport.c +++ b/ports/winnt/ntpd/ntp_iocompletionport.c @@ -61,7 +61,7 @@ static HANDLE WaitableIoEventHandle = NULL; #define MAXHANDLES 3 HANDLE WaitHandles[MAXHANDLES] = { NULL, NULL, NULL }; -//#define USE_HEAP +#define USE_HEAP IoCompletionInfo * GetHeapAlloc(char *fromfunc) @@ -76,7 +76,7 @@ GetHeapAlloc(char *fromfunc) lpo = (IoCompletionInfo *) calloc(1, sizeof(IoCompletionInfo)); #endif #ifdef DEBUG - if (debug > 1) { + if (debug > 3) { printf("Allocation %d memory for %s, ptr %x\n", sizeof(IoCompletionInfo), fromfunc, lpo); } #endif @@ -87,7 +87,7 @@ void FreeHeap(IoCompletionInfo *lpo, char *fromfunc) { #ifdef DEBUG - if (debug > 1) + if (debug > 3) { printf("Freeing memory for %s, ptr %x\n", fromfunc, lpo); } @@ -238,7 +238,8 @@ init_io_completion_port( */ WaitableIoEventHandle = CreateEvent(NULL, FALSE, FALSE, "WaitableIoEventHandle"); if (WaitableIoEventHandle == NULL) { - msyslog(LOG_ERR, "Can't create I/O event handle: %m"); + msyslog(LOG_ERR, + "Can't create I/O event handle: %m - another process may be running - EXITING"); exit(1); } @@ -253,9 +254,9 @@ init_io_completion_port( /* * Initialize the Wait Handles */ - WaitHandles[0] = CreateEvent(NULL, FALSE, FALSE, "WaitHandles0"); /* exit request */ - WaitHandles[1] = get_timer_handle(); - WaitHandles[2] = get_io_event(); + WaitHandles[0] = get_io_event(); + WaitHandles[1] = CreateEvent(NULL, FALSE, FALSE, "WaitHandles0"); /* exit request */ + WaitHandles[2] = get_timer_handle(); /* Have one thread servicing I/O - there were 4, but this would * somehow cause NTP to stop replying to ntpq requests; TODO @@ -343,19 +344,19 @@ OnIoReadComplete(DWORD i, IoCompletionInfo *lpo, DWORD Bytes, int errstatus) buff->dstadr = NULL; buff->recv_srcclock = rio->srcclock; add_full_recv_buffer(buff); + if( !SetEvent( WaitableIoEventHandle ) ) { +#ifdef DEBUG + if (debug > 3) { + printf( "Error %d setting IoEventHandle\n", GetLastError() ); + } +#endif + } } else { freerecvbuf(buff); } } - if( !SetEvent( WaitableIoEventHandle ) ) { -#ifdef DEBUG - if (debug > 3) { - printf( "Error %d setting IoEventHandle\n", GetLastError() ); - } -#endif - } QueueIORead( rio, newbuff, lpo ); return 1; @@ -410,10 +411,11 @@ static unsigned long QueueSocketRecv(SOCKET s, recvbuf_t *buff, IoCompletionInfo DWORD Flags = 0; buff->fd = s; AddrLen = sizeof(struct sockaddr_in); + buff->src_addr_len = sizeof(struct sockaddr); if (SOCKET_ERROR == WSARecvFrom(buff->fd, &buff->wsabuff, 1, &BytesReceived, &Flags, - (struct sockaddr *) &buff->recv_srcadr, (LPINT) &AddrLen, + (struct sockaddr *) &buff->recv_srcadr, (LPINT) &buff->src_addr_len, (LPOVERLAPPED) lpo, NULL)) { DWORD Result = WSAGetLastError(); switch (Result) { @@ -459,7 +461,20 @@ OnSocketRecv(DWORD i, IoCompletionInfo *lpo, DWORD Bytes, int errstatus) /* Convert the overlapped pointer back to a recvbuf pointer. */ + /* + * Check returned structures + */ + if (lpo == NULL) + return (1); /* Nothing to do */ + buff = lpo->recv_buf; + /* + * Make sure we have a buffer + */ + if (buff == NULL) { +// FreeHeap(lpo, "OnSocketRecv: Socket Closed"); + return (1); + } /* * If the socket is closed we get an Operation Aborted error @@ -472,7 +487,6 @@ OnSocketRecv(DWORD i, IoCompletionInfo *lpo, DWORD Bytes, int errstatus) return (1); } - get_systime(&buff->recv_time); /* * Get a new recv buffer for the next packet @@ -487,36 +501,36 @@ OnSocketRecv(DWORD i, IoCompletionInfo *lpo, DWORD Bytes, int errstatus) } else { - if (Bytes > 0 && inter->ignore_packets == ISC_FALSE) { + /* + * If we keep it add some info to the structure + */ + if (Bytes > 0 && inter->ignore_packets == ISC_FALSE) { + get_systime(&buff->recv_time); buff->recv_length = (int) Bytes; buff->receiver = receive; buff->dstadr = inter; #ifdef DEBUG if (debug > 3) - printf("Received %d bytes from %s\n", Bytes, stoa(&buff->recv_srcadr)); + printf("Received %d bytes in buffer %x from %s\n", Bytes, buff, stoa(&buff->recv_srcadr)); #endif add_full_recv_buffer(buff); + /* + * Now signal we have something to process + */ + if( !SetEvent( WaitableIoEventHandle ) ) { +#ifdef DEBUG + if (debug > 1) { + printf( "Error %d setting IoEventHandle\n", GetLastError() ); + } +#endif + } } else { freerecvbuf(buff); } } - QueueSocketRecv(inter->fd, newbuff, lpo); - /* - * Now signal we have something to process - */ -#if 0 - if (newbuff != buff) { - if( !SetEvent( WaitableIoEventHandle ) ) { -#ifdef DEBUG - if (debug > 3) { - printf( "Error %d setting IoEventHandle\n", GetLastError() ); - } -#endif - } - } -#endif - + if (newbuff != NULL) + QueueSocketRecv(inter->fd, newbuff, lpo); return 1; } @@ -732,38 +746,38 @@ io_completion_port_write( */ int GetReceivedBuffers() { -// DWORD Index = WaitForMultipleObjects(MAXHANDLES, WaitHandles, FALSE, INFINITE); - DWORD Index = WaitForMultipleObjects(MAXHANDLES, WaitHandles, FALSE, 500); - switch (Index) { - case WAIT_OBJECT_0 + 0 : /* exit request */ - exit(0); - break; - - case WAIT_OBJECT_0 + 1 : /* timer */ - timer(); - break; - - case WAIT_OBJECT_0 + 2 : /* Io event */ + isc_boolean_t have_packet = ISC_FALSE; + while (!have_packet) { + DWORD Index = WaitForMultipleObjects(MAXHANDLES, WaitHandles, FALSE, INFINITE); + switch (Index) { + case WAIT_OBJECT_0 + 0 : /* Io event */ # ifdef DEBUG - if ( debug > 3 ) - { - printf( "IoEvent occurred\n" ); - } + if ( debug > 3 ) + { + printf( "IoEvent occurred\n" ); + } # endif - break; - - case WAIT_IO_COMPLETION : /* loop */ - case WAIT_TIMEOUT : - break; - case WAIT_FAILED: - msyslog(LOG_ERR, "ntpd: WaitForMultipleObjectsEx Failed: Error: %m"); - break; - - /* For now do nothing if not expected */ - default: - break; + have_packet = ISC_TRUE; + break; + case WAIT_OBJECT_0 + 1 : /* exit request */ + exit(0); + break; + case WAIT_OBJECT_0 + 2 : /* timer */ + timer(); + break; + case WAIT_IO_COMPLETION : /* loop */ + case WAIT_TIMEOUT : + break; + case WAIT_FAILED: + msyslog(LOG_ERR, "ntpd: WaitForMultipleObjects Failed: Error: %m"); + break; + + /* For now do nothing if not expected */ + default: + break; - } /* switch */ + } /* switch */ + } return (full_recvbuffs()); /* get received buffers */ }