Correct a bug in the 'allow arbitrary number of socket descriptors' changes

made to the RPC code some months ago. The value of __svc_fdsetsize is being
calculated incorrectly.

Logically, one would assume that __svc_fdsetsize is being used as a
substitute for FD_SETSIZE, with the difference being that __svc_fdsetsize
can be expanded on the fly to accomodate more descriptors if need be.
There are two problems: first, __svc_fdsetsize is not initialized to 0.
Second, __svc_fdsetsize is being calculated in svc.c:xprt_registere() as:

                __svc_fdsetsize = howmany(sock+1, NFDBITS);

This is wrong. If we are adding a socket with index value 4 to the
descriptor set, then __svc_fdsetsize will be 1 (since fds_bits is
an unsigned long, it can support any descriptor from 0 to 31, so we
only need one of them). In order for this to make sense with the
rest of the code though, it should be:

                __svc_fdsetsize = howmany(sock+1, NFDBITS) * NFDBITS;

Now if sock == 4, __svc_fdsetsize will be 32.

This bug causes 2 errors to occur. First, in xprt_register(), it
causes the __svc_fdset descriptor array to be freed and reallocated
unnecessarily. The code checks if it needs to expand the array using
the test: if (sock + 1 > __svc_fdsetsize). The very first time through,
__svc_fdsetsize is 0, which is fine: an array has to be allocated the
first time out. However __svc_fdsetsize is incorrectly set to 1, so
on the second time through, the test (sock + 1 > __svc_fdsetsize)
will still succeed, and the __svc_fdset array will be destroyed and
reallocated for no reason.

Second, the code in svc_run.c:svc_run() can become hopelessly confused.
The svc_run() routine malloc()s its own fd_set array using the value
of __svc_fdsetsize to decide how much memory to allocate. Once the
xprt_register() function expands the __svc_fdset array the first time,
the value for __svc_fdsetsize becomes 2, which is too small: the resulting
calculation causes the code to allocate an array that's only 32 bits wide
when it actually needs 64 bits. It also uses the valuse of __svc_fdsetsize
when copying the contents of the __svc_fdset array into the new array.
The end result is that all but the first 32 file descriptors get lost.

Note: from what I can tell, this bug originated in OpenBSD and was
brought over to us when the code was merged. The bug is still there
in the OpenBSD source.

Total nervous breakdown averted by: Electric Fence 2.0.5

1 files changed, 4 insertions, 4 deletions

diff --git a/lib/libc/rpc/svc.c b/lib/libc/rpc/svc.c
index ab0e8c1..ba0b53f 100644
--- a/lib/libc/rpc/svc.c
+++ b/lib/libc/rpc/svc.c
@@ -30,7 +30,7 @@
 #if defined(LIBC_SCCS) && !defined(lint)
 /*static char *sccsid = "from: @(#)svc.c 1.44 88/02/08 Copyr 1984 Sun Micro";*/
 /*static char *sccsid = "from: @(#)svc.c	2.4 88/08/11 4.0 RPCSRC";*/
-static char *rcsid = "$Id: svc.c,v 1.7 1996/12/30 15:07:33 peter Exp $";
+static char *rcsid = "$Id: svc.c,v 1.12 1997/05/28 05:05:23 wpaul Exp $";
 #endif
 
 /*
@@ -72,8 +72,8 @@ static struct svc_callout {
 
 static struct svc_callout *svc_find();
 
-int __svc_fdsetsize;
-fd_set *__svc_fdset;
+int __svc_fdsetsize = 0;
+fd_set *__svc_fdset = NULL;
 
 /* ***************  SVCXPRT related stuff **************** */
 
@@ -98,7 +98,7 @@ xprt_register(xprt)
 			free(__svc_fdset);
 		}
 		__svc_fdset = fds;
-		__svc_fdsetsize = howmany(sock+1, NFDBITS);
+		__svc_fdsetsize = howmany(sock+1, NFDBITS) * NFDBITS;
 	}
 
 	if (sock < FD_SETSIZE)