From owner-freebsd-current Thu Dec 10 22:56:43 1998 Return-Path: Received: (from majordom@localhost) by hub.freebsd.org (8.8.8/8.8.8) id WAA25463 for freebsd-current-outgoing; Thu, 10 Dec 1998 22:56:43 -0800 (PST) (envelope-from owner-freebsd-current@FreeBSD.ORG) Received: from apollo.backplane.com (apollo.backplane.com [209.157.86.2]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id WAA25457 for ; Thu, 10 Dec 1998 22:56:39 -0800 (PST) (envelope-from dillon@apollo.backplane.com) Received: (from dillon@localhost) by apollo.backplane.com (8.9.1/8.9.1) id WAA35034; Thu, 10 Dec 1998 22:56:24 -0800 (PST) (envelope-from dillon) Date: Thu, 10 Dec 1998 22:56:24 -0800 (PST) From: Matthew Dillon Message-Id: <199812110656.WAA35034@apollo.backplane.com> To: "Jordan K. Hubbard" Cc: Archie Cobbs , jwd@unx.sas.com (John W. DeBoskey), freebsd-current@FreeBSD.ORG Subject: Re: inetd: realloc/free bug References: <3244.913357673@zippy.cdrom.com> Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG I think we can get this fixed pretty quickly, there really isn't much to it. A couple of minor race conditions in select() as Archie pointed out, but there is so little left that can go wrong I expect we'll cover all our bases in short order. My current proposed diff is included below. This is off the top of my head and barely tested (yet), but ... -Matt : :> Ick. character flags through a pipe? Ick. Ick. Let me fix this :> thing, guys, I'm getting tired of all the arguing and non-action for :> such a simple bug. : :You won't get any argument from me. :-) : :- Jordan : Matthew Dillon Engineering, HiWay Technologies, Inc. & BEST Internet Communications & God knows what else. (Please include original email in any response) Index: inetd.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/inetd/inetd.c,v retrieving revision 1.41 diff -c -r1.41 inetd.c *** inetd.c 1998/11/04 19:39:46 1.41 --- inetd.c 1998/12/11 06:54:48 *************** *** 431,449 **** (void)setenv("inetd_dummy", dummy, 1); } for (;;) { int n, ctrl; fd_set readable; if (nsock == 0) { - (void) sigblock(SIGBLOCK); while (nsock == 0) sigpause(0L); - (void) sigsetmask(0L); } readable = allsock; ! if ((n = select(maxsock + 1, &readable, (fd_set *)0, ! (fd_set *)0, (struct timeval *)0)) <= 0) { if (n < 0 && errno != EINTR) { syslog(LOG_WARNING, "select: %m"); sleep(1); --- 431,467 ---- (void)setenv("inetd_dummy", dummy, 1); } + (void) sigblock(SIGBLOCK); + for (;;) { int n, ctrl; fd_set readable; + struct timeval tv = { 5, 0 }; + + + /* + * Handle signal masking and select. Signals are unmasked and + * we pause if we have no active descriptors. If we do have + * active descriptors, leave signals unmasked through the select() + * call. + * + * Signals are masked at all other times. + */ if (nsock == 0) { while (nsock == 0) sigpause(0L); } + + (void) sigsetmask(0L); + + readable = allsock; ! n = select(maxsock + 1, &readable, NULL, NULL, &tv); ! ! (void) sigblock(SIGBLOCK); ! ! if (n <= 0) { if (n < 0 && errno != EINTR) { syslog(LOG_WARNING, "select: %m"); sleep(1); *************** *** 490,496 **** } } else ctrl = sep->se_fd; ! (void) sigblock(SIGBLOCK); pid = 0; dofork = (sep->se_bi == 0 || sep->se_bi->bi_fork); if (dofork) { --- 508,514 ---- } } else ctrl = sep->se_fd; ! /* (void) sigblock(SIGBLOCK); */ pid = 0; dofork = (sep->se_bi == 0 || sep->se_bi->bi_fork); if (dofork) { *************** *** 509,515 **** "%s/%s server failing (looping), service terminated", sep->se_service, sep->se_proto); close_sep(sep); ! sigsetmask(0L); if (!timingout) { timingout = 1; alarm(RETRYTIME); --- 527,533 ---- "%s/%s server failing (looping), service terminated", sep->se_service, sep->se_proto); close_sep(sep); ! /* sigsetmask(0L); */ if (!timingout) { timingout = 1; alarm(RETRYTIME); *************** *** 524,536 **** if (sep->se_accept && sep->se_socktype == SOCK_STREAM) close(ctrl); ! sigsetmask(0L); sleep(1); continue; } if (pid) addchild(sep, pid); ! sigsetmask(0L); if (pid == 0) { if (dofork) { if (debug) --- 542,554 ---- if (sep->se_accept && sep->se_socktype == SOCK_STREAM) close(ctrl); ! /* sigsetmask(0L); */ sleep(1); continue; } if (pid) addchild(sep, pid); ! /* sigsetmask(0L); */ if (pid == 0) { if (dofork) { if (debug) *************** *** 713,719 **** int signo; { struct servtab *sep, *new, **sepp; ! long omask; if (!setconfig()) { syslog(LOG_ERR, "%s: %m", CONFIG); --- 731,737 ---- int signo; { struct servtab *sep, *new, **sepp; ! /* long omask; */ if (!setconfig()) { syslog(LOG_ERR, "%s: %m", CONFIG); *************** *** 751,757 **** int i; #define SWAP(a, b) { typeof(a) c = a; a = b; b = c; } ! omask = sigblock(SIGBLOCK); /* copy over outstanding child pids */ if (sep->se_maxchild && new->se_maxchild) { new->se_numchild = sep->se_numchild; --- 769,775 ---- int i; #define SWAP(a, b) { typeof(a) c = a; a = b; b = c; } ! /* omask = sigblock(SIGBLOCK); */ /* copy over outstanding child pids */ if (sep->se_maxchild && new->se_maxchild) { new->se_numchild = sep->se_numchild; *************** *** 784,790 **** SWAP(sep->se_server, new->se_server); for (i = 0; i < MAXARGV; i++) SWAP(sep->se_argv[i], new->se_argv[i]); ! sigsetmask(omask); freeconfig(new); if (debug) print_service("REDO", sep); --- 802,808 ---- SWAP(sep->se_server, new->se_server); for (i = 0; i < MAXARGV; i++) SWAP(sep->se_argv[i], new->se_argv[i]); ! /* sigsetmask(omask); */ freeconfig(new); if (debug) print_service("REDO", sep); *************** *** 839,845 **** /* * Purge anything not looked at above. */ ! omask = sigblock(SIGBLOCK); sepp = &servtab; while ((sep = *sepp)) { if (sep->se_checked) { --- 857,863 ---- /* * Purge anything not looked at above. */ ! /* omask = sigblock(SIGBLOCK); */ sepp = &servtab; while ((sep = *sepp)) { if (sep->se_checked) { *************** *** 856,862 **** freeconfig(sep); free((char *)sep); } ! (void) sigsetmask(omask); } void --- 874,880 ---- freeconfig(sep); free((char *)sep); } ! /* (void) sigsetmask(omask); */ } void *************** *** 865,873 **** { int i; struct servtab *sepp; ! long omask; ! omask = sigblock(SIGBLOCK); for (sepp = servtab; sepp; sepp = sepp->se_next) { if (sepp == sep) continue; --- 883,891 ---- { int i; struct servtab *sepp; ! /* long omask; */ ! /* omask = sigblock(SIGBLOCK); */ for (sepp = servtab; sepp; sepp = sepp->se_next) { if (sepp == sep) continue; *************** *** 884,890 **** if (sep->se_fd != -1) (void) close(sep->se_fd); sep->se_fd = -1; ! (void) sigsetmask(omask); } void --- 902,908 ---- if (sep->se_fd != -1) (void) close(sep->se_fd); sep->se_fd = -1; ! /* (void) sigsetmask(omask); */ } void *************** *** 997,1003 **** struct servtab *cp; { struct servtab *sep; ! long omask; sep = (struct servtab *)malloc(sizeof (*sep)); if (sep == (struct servtab *)0) { --- 1015,1021 ---- struct servtab *cp; { struct servtab *sep; ! /* long omask; */ sep = (struct servtab *)malloc(sizeof (*sep)); if (sep == (struct servtab *)0) { *************** *** 1006,1015 **** } *sep = *cp; sep->se_fd = -1; ! omask = sigblock(SIGBLOCK); sep->se_next = servtab; servtab = sep; ! sigsetmask(omask); return (sep); } --- 1024,1033 ---- } *sep = *cp; sep->se_fd = -1; ! /* omask = sigblock(SIGBLOCK); */ sep->se_next = servtab; servtab = sep; ! /* sigsetmask(omask); */ return (sep); } *************** *** 1783,1788 **** --- 1801,1809 ---- /* * Based on TCPMUX.C by Mark K. Lottor November 1988 * sri-nic::ps:tcpmux.c + * + * signals are masked on call, we have to unmask SIGALRM for the + * duration of the read. */ *************** *** 1794,1812 **** { int count = 0, n; struct sigaction sa; sa.sa_flags = 0; sigemptyset(&sa.sa_mask); sa.sa_handler = SIG_DFL; sigaction(SIGALRM, &sa, (struct sigaction *)0); do { alarm(10); n = read(fd, buf, len-count); alarm(0); if (n == 0) ! return (count); ! if (n < 0) ! return (-1); while (--n >= 0) { if (*buf == '\r' || *buf == '\n' || *buf == '\0') return (count); --- 1815,1839 ---- { int count = 0, n; struct sigaction sa; + long omask; sa.sa_flags = 0; sigemptyset(&sa.sa_mask); sa.sa_handler = SIG_DFL; sigaction(SIGALRM, &sa, (struct sigaction *)0); + + omask = sigsetmask(SIGBLOCK & ~sigmask(SIGALRM)); + do { alarm(10); n = read(fd, buf, len-count); alarm(0); if (n == 0) ! break; ! if (n < 0) { ! count = -1; ! break; ! } while (--n >= 0) { if (*buf == '\r' || *buf == '\n' || *buf == '\0') return (count); *************** *** 1814,1819 **** --- 1841,1848 ---- buf++; } } while (count < len); + + sigsetmask(omask); return (count); } To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message