svn commit: r265051 - head/sys/sys

Bruce Evans brde at optusnet.com.au
Tue Apr 29 00:02:52 UTC 2014


On Mon, 28 Apr 2014, Ed Maste wrote:

> Log:
>  Drop explicit unsigned from FD_SETSIZE constant
>
>  FD_SETSIZE is often used as an argument to select or compared with an
>  integer file descriptor.  Rather than force 3rd party software to add
>  explicit casts, just make it a plain (int) constant as on other
>  operating systems.
>
>  Previous discussion:
>  http://lists.freebsd.org/pipermail/freebsd-standards/2012-July/002410.html

Thanks.

The above discussion refers to my mail in 2002 about related things.
I just debugged why this is (now) a non-problem for poll().  It is
because the design error of using an unsigned type for a count is
standard for poll(), so the unwanted promotions to unsigned should
already be expected and handled for poll().  They mainly give confusing
differences from select().

% #ifndef _SYS_SYSPROTO_H_
% struct poll_args {
% 	struct pollfd *fds;
% 	u_int	nfds;
% 	int	timeout;
% };
% #endif

nfds actually has type nfds_t, but nfds_t is just u_int.  POSIX
specificies that nfsd_t is an unsigned integer type.

% int
% sys_poll(td, uap)
% 	struct thread *td;
% 	struct poll_args *uap;
% {
% 	struct pollfd *bits;
% 	struct pollfd smallbits[32];
% 	sbintime_t asbt, precision, rsbt;
% 	u_int nfds;

As mentioned in old mail:
     The nfsds arg has always had type u_int in FreeBSD.  The bounds
     checking of the nfds arg worked between 1997 and 2001 since it was
     done directly on the arg.  However, it was broken for 3 weeks in
     Feb 2001, by introducing this local variable nfds and giving it a
     wrong type (int).  This gave overflow for negative nfds.  Other
     BSDs apparently had this bug in poll() and/or select() for longer.

% 	int error;
% 	size_t ni;
% 
% 	nfds = uap->nfds;
% 	if (nfds > maxfilesperproc && nfds > FD_SETSIZE) 
% 		return (EINVAL);

     The "fix" in Feb 2001 was to bogusly cast "int nfds" back to the
     correct type u_int.  The behaviour on overflow was stll undefined,
     but was benign.  Then in Feb 2002, breaking the type of FD_SETSIZE
     to u_int would have given the same bogus conversion.  Then in Jun
     2002, the type of the local variable was fixed (modulo the
     assumption than nfds_t == u_int).

So except in the 3 week broken period, we just exploit the arg type
being bogusly unsigned.  Promotions still occur, but are harmless.
     (maxfilesperproc has the correct type (int), so it gets promoted
     to the type of nfds (u_int) in the first comparison.  Promotion
     preserves its value, except possibly on weird arches where
     maxfilesperproc is weirdly large.  So the comparison works.  Now,
     a similar promotion occurs for FD_SETSIZE, and any weirdness in
     the first comparison would be compensated for by the second
     comparison, provided FD_SETSIZE is not also weirdly large.)

Bruce


More information about the svn-src-head mailing list