uses readtcp() to gather data from the network; readtcp() uses select(),
with a timeout of 35 seconds. The problem with this is that if you
connect to a TCP server, send two bytes of data, then just pause, the
server will remain blocked in readtcp() for up to 35 seconds, which is
sort of a long time. If you keep doing this every 35 seconds, you can
keep the server occupied indefinitely.
To fix this, I modified readtcp() (and its cousin, readunix() in svc_unix.c)
to monitor all service transport handles instead of just the current socket.
This allows the server to keep handling new connections that arrive while
readtcp() is running. This prevents one client from potentially monopolizing
a server.
Also, while I was here, I fixed a bug in the timeout calculations. Someone
attempted to adjust the timeout so that if select() returned EINTR and the
loop was restarted, the timeout would be reduced so that rather than waiting
for another 35 seconds, you could never wait for more than 35 seconds total.
Unfortunately, the calculation was wrong, and the timeout could expire much
sooner than 35 seconds.
recently in BUGTRAQ. If a stream oriented transport fails to properly decode
an RPC message header structure where there should be one, it should mark
the stream as dead so that the connection will be dropped.
partway through its attempt to decode the result structure sent by
the server. If this happens, it can leave the result partially
populated with dynamically allocated memory. In this event, the
xdr_replymsg() failure is detected and RPC_CANTDECODERES is returned,
but the memory in the partially populated result struct is not
free()d.
The end result is that memory is leaked when an RPC_CANTDECODERES
error occurs. (This condition can occur if a CLIENT * handle is created
using clntudp_bufcreate() with a receive buffer size that is too small
to handle the result sent by the server.)
Fixed by setting reply_xdrs.x_op to XDR_FREE and calling
xdr_replymsg() again to free the memory if an RPC_CANTDECODERES error
is detected.
I suspect that the clnt_tcp.c, clnt_unix.c and clnt_raw.c modules
may ha a similar problem, but I haven't duplicated the condition with
those yet.
Found by: dbmalloc
to fail under certain circumstances.
1. In one spot, the ifr_flags member was being examined in the
wrong structure, thus it contained garbage. On a machine in which
only the loopback interface was up, this caused everything that
wanted to talk to the portmapper to fail -- a particular problem
with laptops, where the pccard ethernet interface is likely to come
up long after the attempt to start mountd, nfsd, amd, etc.
2. Compounding the above problem, get_myaddress() returned a
successful status even though it failed to find an address that it
considered good enough.
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
undefined symbol referenced from libc. Without the stub, it is
impossible to execute any program using the shared library if
LD_BIND_NOW=1 is in the environment. The stub always returns
failure, but it can be overridden outside the library when necessary.
I don't know whether this is the "correct" fix, but it is intolerable
to have any undefined symbols referenced from libc.
The logic in get_myaddress() is broken: it always returns the loopback
address due to the following rule:
if ((ifreq.ifr_flags & IFF_UP) &&
ifr->ifr_addr.sa_family == AF_INET &&
(loopback == 1 && (ifreq.ifr_flags & IFF_LOOPBACK))) {
The idea is that we want to select the interface address only if it's
up and it's in the AF_INET family. If it turns uout we don't have
such an interface available, we make a second pass through the loop,
this time settling for the loopback interface. But the logic inadvertently
locks out all cases when loopback == 0, so nothing is ever selected until
the second pass (when loopback == 1).
This is changed to:
if (((ifreq.ifr_flags & IFF_UP) &&
ifr->ifr_addr.sa_family == AF_INET) ||
(loopback == 1 && (ifreq.ifr_flags & IFF_LOOPBACK))) {
which I think does the right thing.
This is yet another bogon I discovered during NIS+ testing; I need
get_myaddress() to work correctly so that the callback code in the
client library will work.
- bde's change to includes section in getrpcent.3
- Lost comment in svc_run.c (the code here was actually the same since
I had fixed the 'fds + 1' bug in my stuff at home before mailing
Peter about it, but I didn't notce that he'd made a change to the
comment right above the changed line).
Also pointed out by the ever vigilant: bde
This concludes tonight's entertainment. Once I'm sure I haven't destroyed
the world with all these changes, I'll import the utilities. Everything
should continue to work as before. If it doesn't let me know.
Special thanks to Mark Murray for running a test 'make world' for me to
shake out the bugs, which, hopefully, I have fixed.
(And there was much rejoicing.)
Note: you'll need to rinstalkl all your includes before compiling libc
the next time you update your sources in order for all this to work.
Reviewed by: Mark Murray
so that all these makefiles can be used to build libc_r too.
Added .if ${LIB} == "c" tests to restrict man page builds to libc
to avoid needlessly building them with libc_r too.
Split libc Makefile into Makefile and Makefile.inc to allow the
libc_r Makefile to include Makefile.inc too.
This will make a number of things easier in the future, as well as (finally!)
avoiding the Id-smashing problem which has plagued developers for so long.
Boy, I'm glad we're not using sup anymore. This update would have been
insane otherwise.
interfaces, until it's redone to use sysctl().
- bump the SIOCGIFCONF buffer size from 1K to 8K
- if we didn't find a suitable address, return a failure. Previously
if it didn't find anything it left the return address uninitialised.
Perhaps it would be better to return AF_INET/111/127.0.0.1 rather than
failing?
(There may be a behavior difference between the 2.1 and 2.2/3.0 kernels
in this area, it seemed to work for me but I have a horribly hacked
select() that might have a bug in the handling of this)
Submitted by: wpaul
Restore the clamp on the return value from rpc_dtablesize().. Some programs
(eg: ypserv) use this as an indication of how large svc_fdset is in their
hand-rolled svc_run() loops. The svc_fdset table is maintained by the
rpc library explicitly for compatability with such programs. (It uses
a different variable-sized bitmap itself internally)
- prototypes now in include files
Obtained from: a diff of FreeBSD vs. OpenBSD/NetBSD rpc code.
Note: potential bug here, It looks like there could be a null pointer
dereference depending on what has already been called to initialise some
shared data.
- kill non-FD_SETSIZE code
Obtained from: a diff of FreeBSD vs. OpenBSD/NetBSD rpc code.
Note, there was a nasty bug with our old code here. It would trash the
stack if a fd > 31 was passed in. It was using a "long" as though it
was an "fd_set", ie: it was assuming that a long was 256 bits wide. :-(
This has been lurking here for a while, since the FD_SETSIZE #ifdef's
were first implemented.
- fix timeout code
- better sequence number generation (for long running daemons)
- dont close an unopen socket
- use standard functions
- 64 bit type safe for wire protocols
- unlimited file descriptors
Obtained from: a diff of FreeBSD vs. OpenBSD/NetBSD rpc code.
