kern/57504: select with large enough bogus fd number set hangs instead of failing with EBADF

[campbell+netbsd%mumble.net@localhost]

>Number:         57504
>Category:       kern
>Synopsis:       select with large enough bogus fd number set hangs instead of failing with EBADF
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jul 06 02:05:00 +0000 2023
>Originator:     Taylor R Campbell
>Release:        current
>Organization:
The SelectBSD Foundation
>Environment:
>Description:
$ cat selwhat.c
#include <sys/poll.h>
#include <sys/select.h>

#include <err.h>
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int
main(int argc, char **argv)
{
	fd_set readfds;
	int fd, ret;

	setprogname(argv[0]);
	if (argc != 2)
		errx(1, "Usage: %s <fd>", getprogname());
	fd = atoi(argv[1]);

	if (fcntl(fd, F_GETFL) != -1 || errno != EBADF)
		errx(1, "fd %d is already open", fd);

	FD_ZERO(&readfds);
	FD_SET(fd, &readfds);
	errno = 0;
	ret = select(fd + 1, &readfds, NULL, NULL, NULL);
	printf("ret=%d\nerrno=%d %s\n", ret, errno, strerror(errno));
	fflush(stdout);
	return ferror(stdout);
}
$ make selwhat
cc -O2   -o selwhat selwhat.c
$ ./selwhat 19
ret=-1
errno=9 Bad file descriptor
$ ./selwhat 20

and it just hangs.

The issue is this logic in revision 1.60 of sys_select.c:

   363          nf = atomic_load_consume(&curlwp->l_fd->fd_dt)->dt_nfiles;
   364          if (nd > nf) {
   365                  /* forgiving; slightly wrong */
   366                  nd = nf;
   367          }

The initial value of dt_nfiles (NDFILE from sys/filedesc.h) is 20, so this code treats select(124, ...) _as if_ it had been select(20, ...).  Finding no fds in {0, 1, 2, ..., 19} set in the fd sets, select chooses to sleep until timeout -- which is to say, forever, in this case.
>How-To-Repeat:

>Fix:
Yes, please!  At the very least, this should be documented if it's intentional.  But it's not clear why this 'forgiving' logic is there at all; is the purpose to avoid large scans when someone calls select(INT_MAX, ...)?