Date: Tue, 29 Apr 2014 09:43:19 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Ed Maste <emaste@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r265051 - head/sys/sys Message-ID: <20140429080705.T1041@besplex.bde.org> In-Reply-To: <201404281342.s3SDgfIO055824@svn.freebsd.org> References: <201404281342.s3SDgfIO055824@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140429080705.T1041>