Received: from snow-white.ee.udel.edu by whimsy.udel.edu id aa18664; 11 Feb 97 10:33 GMT Received: from sunrise.gv.tsc.tdk.com (root@sunrise.gv.tsc.tdk.com [192.168.241.191]) by gatekeeper.tsc.tdk.com (8.8.4/8.8.4) with ESMTP id CAA19140 for ; Tue, 11 Feb 1997 02:33:10 -0800 (PST) Received: from salsa.gv.tsc.tdk.com (salsa.gv.tsc.tdk.com [192.168.241.194]) by sunrise.gv.tsc.tdk.com (8.8.4/8.8.4) with ESMTP id CAA06659 for ; Tue, 11 Feb 1997 02:33:09 -0800 (PST) Received: (from gdonl@localhost) by salsa.gv.tsc.tdk.com (8.8.4/8.8.4) id CAA26417; Tue, 11 Feb 1997 02:33:08 -0800 (PST) From: Don Lewis Message-Id: <199702111033.CAA26417@salsa.gv.tsc.tdk.com> Date: Tue, 11 Feb 1997 02:33:07 -0800 In-Reply-To: stenn@whimsy.udel.edu "Re: xntpd ntp_io.c input_handler reentered unsafely" (Feb 11, 3:07am) X-Mailer: Mail User's Shell (7.2.6 alpha(3) 7/19/95) To: stenn@whimsy.udel.edu Subject: Re: xntpd ntp_io.c input_handler reentered unsafely Cc: Don.Lewis@tsc.tdk.com I studied the code some more, and it has to keep the signal handler rather than reset it to SIG_DFL. Otherwise the SIGALRM handler would only work once. } If you have any suggestions or patches on how to handle things with } xntpd, I'd be happy to receive them. } } 5.89.5 is already out, and there might be another release due out soon } with some more small changes to input_handler in ntp_io.c . Here's a patch vs. 5.89.5. The changes are: configure.in to looks for sigvec() matching change to config.h.in libntp/syssignal.c changed to use the appropriate signal implementation bogus call to sigset() in sigio_handler() removed sigio handler saves and restores errno, otherwise errno could be corrupted outside the signal context set_signal() always just calls signal_no_reset(), the mask twiddling stuff in the sigaction() version was redundant save and restore errno in the debug signal handlers in xntpd/ntpd.c Not changed: configure isn't any smarter about HAVE_SIGNALED_IO reentrant calls to msyslog() are possible since it is called from within the signal handlers and from the part of the loop in worker_thread where signals are not blocked (the debug signals aren't blocked at all). Core dumps could result ... These changes at at least compile under Solaris 2.5.1 and SunOS 4.1.4 ... *** ORIGconfig.h.in Wed Feb 5 00:04:51 1997 --- config.h.in Tue Feb 11 01:38:16 1997 *************** *** 462,467 **** --- 462,470 ---- /* Define if you have the sigaction function. */ #undef HAVE_SIGACTION + /* Define if you have the sigvec function. */ + #undef HAVE_SIGVEC + /* Define if you have the sigset function. */ #undef HAVE_SIGSET *** ORIGconfigure.in Mon Feb 10 17:27:27 1997 --- configure.in Tue Feb 11 01:02:37 1997 *************** *** 284,290 **** AC_CHECK_FUNCS(gettimeofday getuid K_open kvm_open memcpy memmove memset nice) AC_CHECK_FUNCS(nlist ntp_adjtime ntp_gettime plock pututline pututxline rtprio) AC_CHECK_FUNCS(setlinebuf setpgid setpriority setsid settimeofday setvbuf) ! AC_CHECK_FUNCS(sigaction sigset sigsuspend stime strchr sysconf uname updwtmp) AC_CHECK_FUNCS(updwtmpx vsprintf) AC_CACHE_CHECK(number of arguments to gettimeofday(), ac_cv_func_Xettimeofday_nargs, --- 284,290 ---- AC_CHECK_FUNCS(gettimeofday getuid K_open kvm_open memcpy memmove memset nice) AC_CHECK_FUNCS(nlist ntp_adjtime ntp_gettime plock pututline pututxline rtprio) AC_CHECK_FUNCS(setlinebuf setpgid setpriority setsid settimeofday setvbuf) ! AC_CHECK_FUNCS(sigaction sigvec sigset sigsuspend stime strchr sysconf uname updwtmp) AC_CHECK_FUNCS(updwtmpx vsprintf) AC_CACHE_CHECK(number of arguments to gettimeofday(), ac_cv_func_Xettimeofday_nargs, *** libntp/ORIGsyssignal.c Sat Feb 1 00:47:37 1997 --- libntp/syssignal.c Tue Feb 11 01:37:20 1997 *************** *** 47,60 **** } } ! #else /* not HAVE_SIGACTION */ ! RETSIGTYPE signal_no_reset(sig, func) int sig; RETSIGTYPE (*func) P((int)); { ! signal(sig, func); } - #endif /* not HAVE_SIGACTION */ --- 47,102 ---- } } ! #elif HAVE_SIGVEC ! void signal_no_reset(sig, func) int sig; RETSIGTYPE (*func) P((int)); { ! struct sigvec sv; ! int n; ! ! bzero((char *) &sv, sizeof(sv)); ! sv.sv_handler = func; ! n = sigvec(sig, &sv, (struct sigvec *)NULL); ! if (n == -1) { ! perror("sigvec"); ! exit(1); ! } } + #elif HAVE_SIGSET + + void + signal_no_reset(sig, func) + int sig; + RETSIGTYPE (*func) P((int)); + { + int n; + + n = sigset(sig, func); + if (n == -1) { + perror("sigset"); + exit(1); + } + } + + #else + + /* Beware! This implementation resets the signal to SIG_DFL */ + void + signal_no_reset(sig, func) + int sig; + RETSIGTYPE (*func) P((int)); + { + int n; + + n = signal(sig, func); + if (n == -1) { + perror("signal"); + exit(1); + } + } + + #endif *** xntpd/ORIGntp_io.c Mon Feb 10 17:11:19 1997 --- xntpd/ntp_io.c Tue Feb 11 01:40:08 1997 *************** *** 2043,2067 **** sigio_handler(sig) int sig; { l_fp ts; - # ifdef HAVE_SIGSET - /* - * This should not be necessary for a signal previously set with - * sigset(). - */ - # if defined(USE_SIGIO) - if (sigset(SIGIO, sigio_handler) == SIG_ERR) - msyslog(LOG_ERR, "sigio_handler: sigset(SIGIO) failed: %m"); - # endif - # if defined(USE_SIGPOLL) - if (sigset(SIGPOLL, sigio_handler) == SIG_ERR) - msyslog(LOG_ERR, "sigio_handler: sigset(SIGPOLL) failed: %m"); - # endif - # endif /* HAVE_SIGSET */ - get_systime(&ts); (void)input_handler(&ts); } /* --- 2043,2054 ---- sigio_handler(sig) int sig; { + int saved_errno = errno; l_fp ts; get_systime(&ts); (void)input_handler(&ts); + errno = saved_errno; } /* *************** *** 2071,2121 **** static void set_signal() { - int n; - struct sigaction vec; - - sigemptyset(&vec.sa_mask); - # ifdef USE_SIGIO ! sigaddset(&vec.sa_mask, SIGIO); ! # endif # ifdef USE_SIGPOLL ! sigaddset(&vec.sa_mask, SIGPOLL); ! # endif ! vec.sa_flags = 0; ! ! # if defined(USE_SIGIO) ! vec.sa_handler = sigio_handler; ! ! while (1) ! { ! n = sigaction(SIGIO, &vec, NULL); ! if (n == -1 && errno == EINTR) continue; ! break; ! } ! ! if (n == -1) ! { ! perror("sigaction"); ! exit(1); ! } ! # endif ! # if defined(USE_SIGPOLL) ! vec.sa_handler = sigio_handler; ! ! while (1) ! { ! n = sigaction(SIGPOLL, &vec, NULL); ! if (n == -1 && errno == EINTR) continue; ! break; ! } ! ! if (n == -1) ! { ! perror("sigaction"); ! exit(1); ! } ! # endif } void --- 2058,2069 ---- static void set_signal() { # ifdef USE_SIGIO ! (void) signal_no_reset(SIGIO, sigio_handler); ! # endif # ifdef USE_SIGPOLL ! (void) signal_no_reset(SIGPOLL, sigio_handler); ! # endif } void *** xntpd/ORIGntpd.c Wed Feb 5 22:37:17 1997 --- xntpd/ntpd.c Tue Feb 11 02:06:07 1997 *************** *** 828,838 **** --- 828,841 ---- moredebug(sig) int sig; { + int saved_errno = errno; + if (debug < 255) { debug++; msyslog(LOG_DEBUG, "debug raised to %d", debug); } + errno = saved_errno; } /* *************** *** 842,852 **** --- 845,858 ---- lessdebug(sig) int sig; { + int saved_errno = errno; + if (debug > 0) { debug--; msyslog(LOG_DEBUG, "debug lowered to %d", debug); } + errno = saved_errno; } #else /* not DEBUG */ /* *************** *** 856,862 **** --- 862,871 ---- no_debug(sig) int sig; { + int saved_errno = errno; + msyslog(LOG_DEBUG, "xntpd not compiled for debugging (signal %d)", sig); + errno = saved_errno; } #endif /* not DEBUG */ --- Truck