NetBSD Problem Report #57504

From www@netbsd.org  Thu Jul  6 02:03:42 2023
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id E1A001A923D
	for <gnats-bugs@gnats.NetBSD.org>; Thu,  6 Jul 2023 02:03:41 +0000 (UTC)
Message-Id: <20230706020340.BF73D1A923E@mollari.NetBSD.org>
Date: Thu,  6 Jul 2023 02:03:40 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: select with large enough bogus fd number set hangs instead of failing with EBADF
X-Send-Pr-Version: www-1.0

>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, ...)?

Home	PR Database Search