Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 10 Dec 1998 22:56:24 -0800 (PST)
From:      Matthew Dillon <dillon@apollo.backplane.com>
To:        "Jordan K. Hubbard" <jkh@zippy.cdrom.com>
Cc:        Archie Cobbs <archie@whistle.com>, jwd@unx.sas.com (John W. DeBoskey), freebsd-current@FreeBSD.ORG
Subject:   Re: inetd: realloc/free bug 
Message-ID:  <199812110656.WAA35034@apollo.backplane.com>
References:   <3244.913357673@zippy.cdrom.com>

next in thread | previous in thread | raw e-mail | index | archive | help
    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.
    <dillon@backplane.com> (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:<mkl>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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199812110656.WAA35034>