svn commit: r265051 - head/sys/sys

Bruce Evans

2014-04-29 00:03:02 UTC

Post by Ed Maste
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.
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