Sun Mar 8 09:47:28 2020 UTC ()

Pull up following revision(s) (requested by mlelstv in ticket #1515):

	sys/kern/sys_select.c: revision 1.42-1.45

PR/54158: Anthony Mallet: poll(2) does not allow polling all possible fds
(hardcoded limit to 1000 + #<open-fds>). Changed to limit by the max of
the resource limit of open descriptors and the above.

Remove the slop code. Suggested by mrg@

Use the max limit (aka maxfiles or the moral equivalent of OPEN_MAX) which
makes poll(2) align with the Posix documentation (which allows EINVAL if
nfds > OPEN_MAX). From: Anthony Mallet

Add slop of 1000 and explain why.


(martin)

diff -r1.40 -r1.40.2.1 src/sys/kern/sys_select.c

--- src/sys/kern/sys_select.c 2017/06/01 02:45:13	1.40
+++ src/sys/kern/sys_select.c 2020/03/08 09:47:28	1.40.2.1

 @@ -1,14 +1,14 @@
-/*	$NetBSD: sys_select.c,v 1.40 2017/06/01 02:45:13 chs Exp $	*/
+/*	$NetBSD: sys_select.c,v 1.40.2.1 2020/03/08 09:47:28 martin Exp $	*/
 /*-
  * Copyright (c) 2007, 2008, 2009, 2010 The NetBSD Foundation, Inc.
  * All rights reserved.
+ *
  * This code is derived from software contributed to The NetBSD Foundation
  * by Andrew Doran and Mindaugas Rasiukevicius.
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
 @@ -74,27 +74,27 @@
+ *
  * The <object-lock> might be a device driver or another subsystem, e.g.
  * socket or pipe.  This lock is not exported, and thus invisible to this
  * subsystem.  Mainly, synchronisation between selrecord() and selnotify()
  * routines depends on this lock, as it will be described in the comments.
+ *
  * Lock order
+ *
  *	<object-lock> ->
  *		selcluster_t::sc_lock
  */
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sys_select.c,v 1.40 2017/06/01 02:45:13 chs Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sys_select.c,v 1.40.2.1 2020/03/08 09:47:28 martin Exp $");
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/filedesc.h>
 #include <sys/file.h>
 #include <sys/proc.h>
 #include <sys/socketvar.h>
 #include <sys/signalvar.h>
 #include <sys/uio.h>
 #include <sys/kernel.h>
 #include <sys/lwp.h>
 #include <sys/poll.h>
 #include <sys/mount.h>
 @@ -478,35 +478,48 @@ sys___pollts50(struct lwp *l, const stru
 	return pollcommon(retval, SCARG(uap, fds), SCARG(uap, nfds), ts, mask);
+}
 int
 pollcommon(register_t *retval, struct pollfd *u_fds, u_int nfds,
     struct timespec *ts, sigset_t *mask)
+{
 	struct pollfd	smallfds[32];
 	struct pollfd	*fds;
 	int		error;
 	size_t		ni;
-	if (nfds > 1000 + curlwp->l_fd->fd_dt->dt_nfiles) {
+	if (nfds > curlwp->l_proc->p_rlimit[RLIMIT_NOFILE].rlim_max + 1000) {
 		/*
-		 * Either the user passed in a very sparse 'fds' or junk!
+		 * Prevent userland from causing over-allocation.
-		 * The kmem_alloc() call below would be bad news.
+		 * Raising the default limit too high can still cause
-		 * We could process the 'fds' array in chunks, but that
+		 * a lot of memory to be allocated, but this also means
 		 * that the file descriptor array will also be large.
+		 *
 		 * To reduce the memory requirements here, we could
 		 * process the 'fds' array in chunks, but that
 		 * is a lot of code that isn't normally useful.
 		 * (Or just move the copyin/out into pollscan().)
+		 *
 		 * Historically the code silently truncated 'fds' to
 		 * dt_nfiles entries - but that does cause issues.
+		 *
 		 * Using the max limit equivalent to sysctl
 		 * kern.maxfiles is the moral equivalent of OPEN_MAX
 		 * as specified by POSIX.
+		 *
 		 * We add a slop of 1000 in case the resource limit was
 		 * changed after opening descriptors or the same descriptor
 		 * was specified more than once.
 		 */
 		return EINVAL;
+	}
 	ni = nfds * sizeof(struct pollfd);
 	if (ni > sizeof(smallfds))
 		fds = kmem_alloc(ni, KM_SLEEP);
 	else
 		fds = smallfds;
 	error = copyin(u_fds, fds, ni);
 	if (error)
 		goto fail;