To: krb5-bugs@mit.edu
Subject: select(2) if no fds < FD_SETSIZE are available.
Organization: The Fall of Imrryr
User-Agent: nmh-1.0.4 (NetBSD/alpha)
Date: Wed, 07 Jan 2009 15:09:24 -0500
From: Roland Dowdeswell <elric@imrryr.org>
Message-ID: <20090107200924.0F56637D93@arioch.imrryr.org>
Precedence: list
Sender: krb5-bugs-incoming-bounces@PCH.mit.edu
Errors-To: krb5-bugs-incoming-bounces@PCH.mit.edu
Content-Length: 3111

sendto_kdc.c uses select(2) and does not check to see if the fds that
it obtains are less than FD_SETSIZE.  This can cause undefined behaviour
as FD_SET() does not do bounds checking.

Although, this limitation should probably be addressed by using Niels
Provos' libevent, I provide a small patch which will:

	1.  return reasonable errors if the size is returned, and

	2.  increase the limit to DESIRED_FD_SETSIZE which I define
	    to be 8192.

I think that (1) or something like it should be applied.  (2) on
the other hand is quite inelegant.  A better approach should be
used, I just in case it is viewed to be a reasonable short term
fix.

I call the inability to obtain a socket < FD_SETSIZE a permanent
error which we can also change.  It seemed at the time best to
simply fail quickly w/o core dumping.

I also return EMFILE in this case so that the error message returned
is slightly more descriptive than KRB5_KDC_UNREACH.  You may very
well be able to reach KDCs, if you just had a few more fds..

Index: sendto_kdc.c
===================================================================
RCS file: /ms/dev/kerberos/mitkrb5/cvs-dirs/mitkrb5-1.4/mitkrb5/src/lib/krb5/os/sendto_kdc.c,v
retrieving revision 1.1.1.2
diff -u -r1.1.1.2 sendto_kdc.c
--- sendto_kdc.c	16 Aug 2005 19:52:03 -0000	1.1.1.2
+++ sendto_kdc.c	7 Jan 2009 17:21:02 -0000
@@ -28,6 +28,25 @@
  * as necessary.
  */
 
+/*
+ * We start out by upping the size of FD_SETSIZE.  On rational operating
+ * systems, this is simple.  One simply #defines FD_SETSIZE before including
+ * anything else.  Linux of course does not support this because they are
+ * better than that.  So, we special case things...
+ */
+
+#define DESIRED_FD_SETSIZE	8192
+#ifndef linux
+#define FD_SETSIZE	DESIRED_FD_SETSIZE
+#else
+#include <features.h>
+#if (__GLIBC__ > 2) || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 2)
+#include <bits/types.h>
+#undef __FD_SETSIZE
+#define __FD_SETSIZE DESIRED_FD_SETSIZE
+#endif
+#endif
+
 #define NEED_SOCKETS
 #define NEED_LOWLEVEL_IO
 #include "fake-addrinfo.h"
@@ -572,8 +591,16 @@
     dprint("start_connection(@%p)\ngetting %s socket in family %d...", state,
 	   ai->ai_socktype == SOCK_STREAM ? "stream" : "dgram", ai->ai_family);
     fd = socket(ai->ai_family, ai->ai_socktype, 0);
+    if (fd >= FD_SETSIZE) {
+	close(fd);
+	state->err = EMFILE;
+	state->state = FAILED;	/* XXXrcd: hmmm, is this permanent...? */
+	dprint("socket: %m creating with af %d\n", state->err, ai->ai_family);
+	return -1;		/* try other hosts */
+    }
     if (fd == INVALID_SOCKET) {
 	state->err = SOCKET_ERRNO;
+	state->state = FAILED;	/* XXXrcd: hmmm, is this permanent...? */
 	dprint("socket: %m creating with af %d\n", state->err, ai->ai_family);
 	return -1;		/* try other hosts */
     }
@@ -1130,6 +1157,9 @@
     if (sel_state->nfds == 0) {
 	/* No addresses?  */
 	retval = KRB5_KDC_UNREACH;
+	for (host = 0; host < n_conns; host++)
+	    if (conns[host].err == EMFILE)
+		retval = EMFILE;
 	goto egress;
     }
     if (e == 0 || winning_conn < 0) {


--
    Roland Dowdeswell                      http://www.Imrryr.ORG/~elric/