Subject: Re: race in select() ?
To: Charles M. Hannum <abuse@spamalicious.com>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: tech-kern
Date: 10/09/2003 22:56:55
--UlVJffcvxoiEqYs2
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Oct 09, 2003 at 08:19:26PM +0000, Charles M. Hannum wrote:
> 
> This does not, in fact, solve the problem.  Nor does it quite work as you 
> describe.
> 
> First of all, your "false positive" detection will cause the return from 
> select(2) to actually be ignored in the race condition case.  In this case, 
> all your patch is really doing is causing us to loop around again and pick up 
> the new soeckt.

Yes, this is what I meant. We'll select() again, and handle the socket
this time.

> 
> Secondly, there is a race condition in the creation of "allsock_select".  If 
> we read the part of "allsock" containing the file descriptor in question, but 
> were interrupted before writing it to allsock_select, then we will lose the 
> setting of the bit that was done in the signal handler.

Ha, right.
Well, I tried to avoid this but we can mask signals in this section.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 24 ans d'experience feront toujours la difference
--

--UlVJffcvxoiEqYs2
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff

? .gdbinit
? inetd
? inetd.cat8
Index: inetd.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/inetd/inetd.c,v
retrieving revision 1.76
diff -u -r1.76 inetd.c
--- inetd.c	2002/01/21 14:42:28	1.76
+++ inetd.c	2003/10/09 20:56:17
@@ -272,7 +272,7 @@
 int	lflag;
 #endif
 int	nsock, maxsock;
-fd_set	allsock;
+fd_set	allsock, allsock_select;
 int	options;
 int	timingout;
 struct	servent *sp;
@@ -531,8 +531,11 @@
 		    sigpause(0L);
 		(void) sigsetmask(0L);
 	    }
+	    (void) sigblock(SIGBLOCK);
 	    readable = allsock;
-	    if ((n = select(maxsock + 1, &readable, (fd_set *)0,
+	    allsock_select = allsock;
+	    (void) sigsetmask(0L);
+	    if ((n = select(maxsock + 1, &allsock_select, (fd_set *)0,
 		(fd_set *)0, (struct timeval *)0)) <= 0) {
 		    if (n == -1 && errno != EINTR) {
 			syslog(LOG_WARNING, "select: %m");
@@ -542,7 +545,8 @@
 	    }
 	    for (sep = servtab; n && sep; sep = nsep) {
 	    nsep = sep->se_next;
-	    if (sep->se_fd != -1 && FD_ISSET(sep->se_fd, &readable)) {
+	    if (sep->se_fd != -1 && FD_ISSET(sep->se_fd, &readable) &&
+		FD_ISSET(sep->se_fd, &allsock_select)) {
 		n--;
 		if (debug)
 			fprintf(stderr, "someone wants %s\n", sep->se_service);
@@ -770,6 +774,7 @@
 					    sep->se_server, WTERMSIG(status));
 				sep->se_wait = 1;
 				FD_SET(sep->se_fd, &allsock);
+				FD_SET(sep->se_fd, &allsock_select);
 				nsock++;
 				if (debug)
 					fprintf(stderr, "restored %s, fd %d\n",

--UlVJffcvxoiEqYs2--