From: Dave Hart Date: Sat, 27 Jan 2024 00:22:53 +0000 (+0000) Subject: [Bug 3903] lib/isc/win32/strerror.c NTstrerror() is not thread-safe. X-Git-Tag: NTP_4_2_8P18_RC1~23 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a876d4bb872ab1b4afce4074e072676d1d398ce5;p=thirdparty%2Fntp.git [Bug 3903] lib/isc/win32/strerror.c NTstrerror() is not thread-safe. Suppress OpenSSL 3 deprecation warning clutter. Fix problem with statisics files not being generated (broken after 4.2.8p17) Remove unneeded 200ms wait during NT service shutdown. Do not compile unneded bsd_strerror.c on Windows. Reduce HAVE_IO_COMPLETION_PORT #ifdef clutter by moving calls to io_completion_port_remove_socket into close_and_delete_fd_from_list(). Silence warnings building on macOS about incompatible types for timeval.tv_usec. Move massive config parser debug output from debug level 5 to 9. Move yylex() debug output from level 4 to level 10. Change overallocated 1k static lexeme buffer to 128 bytes. bk: 65b44cdd3_DyQ6f9O7DUnyWr057aPQ --- diff --git a/ChangeLog b/ChangeLog index 642fbf686..53e08a76c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ --- +* [Bug 3903] lib/isc/win32/strerror.c NTstrerror() is not thread-safe. + * [Bug 3901] LIB_GETBUF isn't thread-safe. * [Bug 3900] fast_xmit() selects wrong local addr responding to mcast on Windows. @@ -74,6 +76,7 @@ * Quiet local addr change logging when unpeering. * Correct missing arg for %s printf specifier in send_blocking_resp_internal(). +* Suppress OpenSSL 3 deprecation warning clutter. --- (4.2.8p17) 2023/06/06 Released by Harlan Stenn diff --git a/include/ntpd.h b/include/ntpd.h index 28e2d05cd..72cecb5bd 100644 --- a/include/ntpd.h +++ b/include/ntpd.h @@ -156,7 +156,8 @@ extern void block_io_and_alarm (void); # define UNBLOCK_IO_AND_ALARM() do {} while (0) # define BLOCK_IO_AND_ALARM() do {} while (0) #endif -#define latoa(pif) localaddrtoa(pif) +#define eptoa(pif) localaddrtoa(pif) +#define latoa(pif) eptoa(pif) extern const char * localaddrtoa(endpt *); #ifdef DEBUG extern const char * iflags_str(u_int32 iflags); diff --git a/libntp/lib/isc/win32/strerror.c b/libntp/lib/isc/win32/strerror.c index 823e9c1b2..21bdad977 100644 --- a/libntp/lib/isc/win32/strerror.c +++ b/libntp/lib/isc/win32/strerror.c @@ -29,6 +29,8 @@ #include #include +#include "lib_strbuf.h" + /* * Forward declarations */ @@ -112,14 +114,15 @@ FormatError(int error) { *pch = '\0'; } - /* strip any trailing CR/LF */ + /* strip any trailing CR/LF and spaces */ if (lpMsgBuf != NULL) { last = strlen(lpMsgBuf); if (last > 0) { --last; } while ('\n' == lpMsgBuf[last] || - '\r' == lpMsgBuf[last]) { + '\r' == lpMsgBuf[last] || + ' ' == lpMsgBuf[last]) { lpMsgBuf[last] = '\0'; if (last > 0) { @@ -140,27 +143,31 @@ char * NTstrerror(int err, BOOL *bfreebuf) { char *retmsg = NULL; - /* Copy the error value first in case of other errors */ - DWORD errval = err; - *bfreebuf = FALSE; /* Get the Winsock2 error messages */ - if (errval >= WSABASEERR && errval <= (WSABASEERR + 1999)) { - retmsg = GetWSAErrorMessage(errval); - if (retmsg != NULL) - return (retmsg); + /* DLH this may not be needed, FormatError/FormatMessage may handle Winsock error codes */ + if (err >= WSABASEERR && err <= (WSABASEERR + 1999)) { + retmsg = GetWSAErrorMessage(err); } /* * If it's not one of the standard Unix error codes, * try a system error message */ - if (errval > (DWORD) _sys_nerr) { - *bfreebuf = TRUE; - return (FormatError(errval)); - } else { - return (strerror(errval)); + if (NULL == retmsg) { + if (err > _sys_nerr) { + *bfreebuf = TRUE; + retmsg = FormatError(err); + } else { + retmsg = lib_getbuf(); + if (0 != strerror_s(retmsg, LIB_BUFLENGTH, err)) { + snprintf(retmsg, LIB_BUFLENGTH, + "Unknown error number %d/0x%x", + err, err); + } + } } + return retmsg; } /* diff --git a/libntp/work_thread.c b/libntp/work_thread.c index 75cc365eb..0d15c6e36 100644 --- a/libntp/work_thread.c +++ b/libntp/work_thread.c @@ -376,7 +376,7 @@ send_blocking_resp_internal( { # ifdef WORK_PIPE if (1 != write(c->resp_write_pipe, "", 1)) - msyslog(LOG_WARNING, "async resolver: blocking_get%sinfo", + msyslog(LOG_WARNING, "async resolver: blocking_get%sinfo" " failed to notify main thread!", (BLOCKING_GETNAMEINFO == resp->rtype) ? "name" diff --git a/ntpd/ntp_config.c b/ntpd/ntp_config.c index 57ecab753..a0b553e85 100644 --- a/ntpd/ntp_config.c +++ b/ntpd/ntp_config.c @@ -2469,7 +2469,7 @@ config_monitor( /* Set the statistics directory */ if (ptree->stats_dir) { - stats_config(STATS_STATSDIR, ptree->stats_dir, 0); + stats_config(STATS_STATSDIR, ptree->stats_dir, TRUE); } /* NOTE: @@ -3361,10 +3361,13 @@ config_nic_rules( if_name = estrdup(if_name); switch (curr_node->match_class) { -#ifdef DEBUG default: +#ifdef DEBUG fatal_error("config_nic_rules: match-class-token=%d", curr_node->match_class); #endif + case T_All: + match_type = MATCH_ALL; + break; case 0: /* @@ -3395,10 +3398,6 @@ config_nic_rules( } break; - case T_All: - match_type = MATCH_ALL; - break; - case T_Ipv4: match_type = MATCH_IPV4; break; @@ -3413,8 +3412,8 @@ config_nic_rules( } switch (curr_node->action) { -#ifdef DEBUG default: +#ifdef DEBUG fatal_error("config_nic_rules: action-token=%d", curr_node->action); #endif case T_Listen: @@ -5194,13 +5193,13 @@ getconfig( } cfgt.source.value.s = estrdup(alt_config_file); #endif /* SYS_WINNT */ - } else + } else { cfgt.source.value.s = estrdup(config_file); - + } /*** BULK OF THE PARSER ***/ #ifdef DEBUG - yydebug = !!(debug >= 5); + yydebug = (debug >= 9); #endif yyparse(); lex_drop_stack(); diff --git a/ntpd/ntp_io.c b/ntpd/ntp_io.c index 113fb42c9..e1db13749 100644 --- a/ntpd/ntp_io.c +++ b/ntpd/ntp_io.c @@ -325,7 +325,7 @@ static endpt * find_addr_in_list (sockaddr_u *); static endpt * find_flagged_addr_in_list(sockaddr_u *, u_int32); static void delete_addr_from_list (sockaddr_u *); static void delete_interface_from_list(endpt *); -static void close_and_delete_fd_from_list(SOCKET); +static void close_and_delete_fd_from_list(SOCKET, endpt *); static void add_addr_to_list (sockaddr_u *, endpt *); static void create_wildcards (u_short); static endpt * findlocalinterface (sockaddr_u *, int, int); @@ -645,7 +645,7 @@ add_asyncio_reader( } /* - * remove asynchio_reader + * remove asyncio_reader */ static void remove_asyncio_reader( @@ -657,9 +657,9 @@ remove_asyncio_reader( UNLINK_SLIST(unlinked, asyncio_reader_list, reader, link, struct asyncio_reader); - if (reader->fd != INVALID_SOCKET) - close_and_delete_fd_from_list(reader->fd); - + if (reader->fd != INVALID_SOCKET) { + close_and_delete_fd_from_list(reader->fd, NULL); + } reader->fd = INVALID_SOCKET; } #endif /* !defined(HAVE_IO_COMPLETION_PORT) && defined(HAS_ROUTING_SOCKET) */ @@ -1024,10 +1024,7 @@ remove_interface( ep->sent, ep->notsent, current_time - ep->starttime); -# ifdef HAVE_IO_COMPLETION_PORT - io_completion_port_remove_socket(ep->fd, ep); -# endif - close_and_delete_fd_from_list(ep->fd); + close_and_delete_fd_from_list(ep->fd, ep); ep->fd = INVALID_SOCKET; } @@ -1035,10 +1032,7 @@ remove_interface( msyslog(LOG_INFO, "stop listening for broadcasts to %s on interface #%d %s", stoa(&ep->bcast), ep->ifnum, ep->name); -# ifdef HAVE_IO_COMPLETION_PORT - io_completion_port_remove_socket(ep->bfd, ep); -# endif - close_and_delete_fd_from_list(ep->bfd); + close_and_delete_fd_from_list(ep->bfd, ep); ep->bfd = INVALID_SOCKET; } # ifdef HAVE_IO_COMPLETION_PORT @@ -2837,10 +2831,7 @@ io_unsetbclient(void) msyslog(LOG_INFO, "stop listening for broadcasts to %s on interface #%d %s", stoa(&ep->bcast), ep->ifnum, ep->name); -# ifdef HAVE_IO_COMPLETION_PORT - io_completion_port_remove_socket(ep->bfd, ep); -# endif - close_and_delete_fd_from_list(ep->bfd); + close_and_delete_fd_from_list(ep->bfd, ep); ep->bfd = INVALID_SOCKET; } ep->flags &= ~INT_BCASTOPEN; @@ -4274,13 +4265,13 @@ calc_addr_distance( const sockaddr_u * a2 ) { - u_char *pdist; - u_char *p1; - u_char *p2; - size_t cb; - int different; - int a1_greater; - u_int u; + u_char * pdist; + const u_char * p1; + const u_char * p2; + size_t cb; + int different; + int a1_greater; + u_int u; REQUIRE(AF(a1) == AF(a2)); @@ -4288,13 +4279,13 @@ calc_addr_distance( AF(dist) = AF(a1); if (IS_IPV4(a1)) { - pdist = (u_char *)&NSRCADR(dist); - p1 = (u_char *)&NSRCADR(a1); - p2 = (u_char *)&NSRCADR(a2); + pdist = ( u_char *)&NSRCADR(dist); + p1 = (const u_char *)&NSRCADR(a1); + p2 = (const u_char *)&NSRCADR(a2); } else { - pdist = (u_char *)&NSRCADR(dist); - p1 = (u_char *)&NSRCADR(a1); - p2 = (u_char *)&NSRCADR(a2); + pdist = ( u_char *)&NSRCADR(dist); + p1 = (const u_char *)&NSRCADR(a1); + p2 = (const u_char *)&NSRCADR(a2); } cb = SIZEOF_INADDR(AF(dist)); different = FALSE; @@ -4566,7 +4557,7 @@ io_closeclock( # ifdef HAVE_IO_COMPLETION_PORT io_completion_port_remove_clock_io(rio); # endif - close_and_delete_fd_from_list(rio->fd); + close_and_delete_fd_from_list(rio->fd, NULL); purge_recv_buffers_for_fd(rio->fd); rio->fd = -1; } @@ -4601,7 +4592,7 @@ kill_asyncio( maxactivefd = 0; while (fd_list != NULL) - close_and_delete_fd_from_list(fd_list->fd); + close_and_delete_fd_from_list(fd_list->fd, NULL); UNBLOCKIO(); } @@ -4629,7 +4620,8 @@ add_fd_to_list( static void close_and_delete_fd_from_list( - SOCKET fd + SOCKET fd, + endpt *ep /* req. if fd is in struct endpt */ ) { vsock_t *lsock; @@ -4643,6 +4635,11 @@ close_and_delete_fd_from_list( switch (lsock->type) { case FD_TYPE_SOCKET: + #ifdef HAVE_IO_COMPLETION_PORT + if (ep != NULL) { + io_completion_port_remove_socket(fd, ep); + } + #endif closesocket(lsock->fd); break; diff --git a/ntpd/ntp_peer.c b/ntpd/ntp_peer.c index 4252770e4..69c1eff45 100644 --- a/ntpd/ntp_peer.c +++ b/ntpd/ntp_peer.c @@ -647,6 +647,24 @@ set_peerdstadr( if (p->dstadr == dstadr) return; + /* + * Do not change the local address of a link-local + * peer address. + */ + if ( p->dstadr != NULL && is_linklocal(&p->dstadr->sin) + && dstadr != NULL) { + return; + } + + /* + * Do not set the local address for a link-local IPv6 peer + * to one with a different scope ID. + */ + if ( dstadr != NULL && IS_IPV6(&p->srcadr) + && SCOPE(&dstadr->sin) != SCOPE(&p->srcadr)) { + return; + } + /* * Don't accept updates to a separate multicast receive-only * endpt while a BCLNT peer is running its unicast protocol. @@ -661,11 +679,12 @@ set_peerdstadr( p->dstadr->peercnt--; UNLINK_SLIST(unlinked, p->dstadr->peers, p, ilink, struct peer); - if (!IS_MCAST(&p->srcadr) &&!(FLAG_DISABLED & p->flags)) { - msyslog(LOG_INFO, "%s local addr %s -> %s", - stoa(&p->srcadr), latoa(p->dstadr), - latoa(dstadr)); - } + } + if ( !IS_MCAST(&p->srcadr) && !(FLAG_DISABLED & p->flags) + && !initializing) { + msyslog(LOG_INFO, "%s local addr %s -> %s", + stoa(&p->srcadr), eptoa(p->dstadr), + eptoa(dstadr)); } p->dstadr = dstadr; @@ -759,13 +778,6 @@ refresh_all_peerinterfaces(void) if (p->dstadr != NULL && (p->reach & 0x3)) { continue; } - /* - * Do not change the local address of a link-local - * peer address. - */ - if (p->dstadr != NULL && is_linklocal(&p->srcadr)) { - continue; - } peer_refresh_interface(p); } } @@ -944,12 +956,22 @@ newpeer( set_peerdstadr(peer, select_peerinterface(peer, srcadr, dstadr)); + /* + * Zero for minpoll or maxpoll means use defaults. + */ + peer->maxpoll = (0 == maxpoll) + ? NTP_MAXDPOLL + : maxpoll; + peer->minpoll = (0 == minpoll) + ? NTP_MINDPOLL + : minpoll; + /* * Clamp maxpoll and minpoll within NTP_MINPOLL and NTP_MAXPOLL, * and further clamp minpoll less than or equal maxpoll. */ - peer->maxpoll = CLAMP(maxpoll, NTP_MINPOLL, NTP_MAXPOLL); - peer->minpoll = CLAMP(minpoll, NTP_MINPOLL, peer->maxpoll); + peer->maxpoll = CLAMP(peer->maxpoll, NTP_MINPOLL, NTP_MAXPOLL); + peer->minpoll = CLAMP(peer->minpoll, NTP_MINPOLL, peer->maxpoll); if (peer->dstadr != NULL) { DPRINTF(3, ("newpeer(%s): using fd %d and our addr %s\n", diff --git a/ntpd/ntp_scanner.c b/ntpd/ntp_scanner.c index 08c203c63..5b0b7f088 100644 --- a/ntpd/ntp_scanner.c +++ b/ntpd/ntp_scanner.c @@ -34,7 +34,7 @@ * ------------------------ */ -#define MAX_LEXEME (1024 + 1) /* The maximum size of a lexeme */ +#define MAX_LEXEME 128 /* The maximum size of a lexeme */ char yytext[MAX_LEXEME]; /* Buffer for storing the input text/lexeme */ u_int32 conf_file_sum; /* Simple sum of characters read */ @@ -278,7 +278,7 @@ lex_close( */ static struct FILE_INFO * -_drop_stack_do( +drop_stack_do( struct FILE_INFO * head ) { @@ -322,7 +322,7 @@ lex_init_stack( void lex_drop_stack() { - lex_stack = _drop_stack_do(lex_stack); + lex_stack = drop_stack_do(lex_stack); } /* Flush the lexer input stack: This will nip all input objects on the @@ -341,7 +341,7 @@ lex_flush_stack() if (NULL != lex_stack) { retv = !lex_stack->force_eof; lex_stack->force_eof = TRUE; - lex_stack->st_next = _drop_stack_do( + lex_stack->st_next = drop_stack_do( lex_stack->st_next); } return retv; @@ -910,9 +910,9 @@ yylex(void) normal_return: if (T_EOC == token) - DPRINTF(4,("\t\n")); + DPRINTF(10, ("\t\n")); else - DPRINTF(4, ("yylex: lexeme '%s' -> %s\n", yytext, + DPRINTF(10, ("yylex: lexeme '%s' -> %s\n", yytext, token_name(token))); if (!yylval_was_set) @@ -921,6 +921,10 @@ normal_return: return token; lex_too_long: + /* + * DLH: What is the purpose of the limit of 50? + * Is there any reason for yytext[] to be bigger? + */ yytext[min(sizeof(yytext) - 1, 50)] = 0; msyslog(LOG_ERR, "configuration item on line %d longer than limit of %lu, began with '%s'", diff --git a/ports/winnt/include/ntp_iocpltypes.h b/ports/winnt/include/ntp_iocpltypes.h index 502237892..836cb6000 100644 --- a/ports/winnt/include/ntp_iocpltypes.h +++ b/ports/winnt/include/ntp_iocpltypes.h @@ -139,7 +139,7 @@ struct IoCtx { l_fp DCDSTime; /* PPS-hack: time of DCD ON */ l_fp FlagTime; /* time stamp of flag/event char*/ l_fp RecvTime; /* time stamp of callback */ - DWORD com_events; /* buffer for COM events */ + DWORD com_events; /* bitmask of serial events */ u_int flTsDCDS : 1; /* DCDSTime valid? */ u_int flTsFlag : 1; /* FlagTime valid? */ } aux; diff --git a/ports/winnt/ntpd/ntservice.c b/ports/winnt/ntpd/ntservice.c index 9b2807907..31762608d 100644 --- a/ports/winnt/ntpd/ntservice.c +++ b/ports/winnt/ntpd/ntservice.c @@ -219,10 +219,8 @@ ntservice_systemisshuttingdown(void) void ntservice_exit(void) { - uninit_io_completion_port(); peer_cleanup(); - Sleep( 200 ); //##++ - + uninit_io_completion_port(); reset_winnt_time(); msyslog(LOG_INFO, "ntservice: The Network Time Protocol Service is stopping."); diff --git a/ports/winnt/vs2015/libntp/libntp.vcxproj b/ports/winnt/vs2015/libntp/libntp.vcxproj index c028b2b18..1b87cea1d 100644 --- a/ports/winnt/vs2015/libntp/libntp.vcxproj +++ b/ports/winnt/vs2015/libntp/libntp.vcxproj @@ -126,7 +126,6 @@ - diff --git a/ports/winnt/vs2015/libntp/libntp.vcxproj.filters b/ports/winnt/vs2015/libntp/libntp.vcxproj.filters index d2ee93b1d..23fff905d 100644 --- a/ports/winnt/vs2015/libntp/libntp.vcxproj.filters +++ b/ports/winnt/vs2015/libntp/libntp.vcxproj.filters @@ -344,9 +344,6 @@ libisc Source Files - - Source Files - diff --git a/sntp/m4/ntp_openssl.m4 b/sntp/m4/ntp_openssl.m4 index 2726eafec..6a2983b50 100644 --- a/sntp/m4/ntp_openssl.m4 +++ b/sntp/m4/ntp_openssl.m4 @@ -267,7 +267,7 @@ case "$with_crypto" in for i in $ntp_ssl_incdir_search do AC_MSG_NOTICE([Searching for openssl/evp.h in $i]) - CPPFLAGS="$NTPSSL_SAVED_CPPFLAGS $i" + CPPFLAGS="$NTPSSL_SAVED_CPPFLAGS -I$i" dnl force uncached AC_CHECK_HEADER AS_UNSET([ac_cv_header_openssl_evp_h]) AC_CHECK_HEADER( @@ -535,6 +535,14 @@ case "$ntp_openssl" in VER_SUFFIX=o AC_CHECK_HEADERS([openssl/cmac.h openssl/hmac.h]) AC_DEFINE([OPENSSL], [], [Use OpenSSL?]) + dnl OpenSSL 3 deprecates a bunch of functions used by Autokey. + dnl Adapting our code to the bold new way is not a priority + dnl for us because we do not want to require OpenSSL 3 yet. + dnl The deprecation warnings clutter up the build output + dnl encouraging the habit of ignoring warninis. + dnl So, tell it to the hand, OpenSSL deprecation warnings... + AC_DEFINE([OPENSSL_SUPPRESS_DEPRECATED], [1], + [Suppress OpenSSL 3 deprecation warnings]) dnl We don't want -Werror for the EVP_MD_do_all_sorted check CFLAGS="$NTPSSL_SAVED_CFLAGS" AC_CHECK_FUNCS([EVP_MD_do_all_sorted]) diff --git a/tests/bug-2803/bug-2803.c b/tests/bug-2803/bug-2803.c index df9dcdec7..ba761c430 100644 --- a/tests/bug-2803/bug-2803.c +++ b/tests/bug-2803/bug-2803.c @@ -7,7 +7,6 @@ #include #include "unity.h" -//#include "bug-2803.h" /* microseconds per second */ #define MICROSECONDS 1000000 @@ -53,10 +52,10 @@ static int do_test( struct timeval timetv, struct timeval tvlast ) if ( failed || verbose ) printf( "timetv %lli|%07li, tvlast %lli|%07li: tvdiff_old: %lli|%07li -> %i, tvdiff_new: %lli|%07li -> %i, same cond: %s\n", - (long long) timetv.tv_sec, timetv.tv_usec, - (long long) tvlast.tv_sec, tvlast.tv_usec, - (long long) tvdiff_old.tv_sec, tvdiff_old.tv_usec, cond_old, - (long long) tvdiff_new.tv_sec, tvdiff_new.tv_usec, cond_new, + (long long) timetv.tv_sec, (u_long)timetv.tv_usec, + (long long) tvlast.tv_sec, (u_long)tvlast.tv_usec, + (long long) tvdiff_old.tv_sec, (u_long)tvdiff_old.tv_usec, cond_old, + (long long) tvdiff_new.tv_sec, (u_long)tvdiff_new.tv_usec, cond_new, failed ? "NO <<" : "yes" ); return failed ? -1 : 0; diff --git a/tests/libntp/timevalops.c b/tests/libntp/timevalops.c index 0c498ca39..4dff90c24 100644 --- a/tests/libntp/timevalops.c +++ b/tests/libntp/timevalops.c @@ -119,9 +119,9 @@ AssertTimevalClose(const struct timeval m, const struct timeval n, const struct printf("m_expr which is %lld.%06lu \nand\n" "n_expr which is %lld.%06lu\nare not close; diff=%lld.%06luusec\n", - (long long)m.tv_sec, m.tv_usec, - (long long)n.tv_sec, n.tv_usec, - (long long)diff.tv_sec, diff.tv_usec); + (long long)m.tv_sec, (u_long)m.tv_usec, + (long long)n.tv_sec, (u_long)n.tv_usec, + (long long)diff.tv_sec, (u_long)diff.tv_usec); return FALSE; }