Re: [PATCH] Fix poll() nfds check.

[Vadim Lobanov]

On Wed, 5 Jul 2006, Andrew Morton wrote:

> On Wed, 5 Jul 2006 20:00:34 -0700 (PDT)
> Vadim Lobanov <vlobanov@xxxxxxxxxxxxx> wrote:
>
> > diff -Npru linux-2.6.17-git25/fs/select.c linux-new/fs/select.c
> > --- linux-2.6.17-git25/fs/select.c	2006-07-05 19:06:56.000000000 -0700
> > +++ linux-new/fs/select.c	2006-07-05 19:10:51.000000000 -0700
> > @@ -671,7 +671,7 @@ int do_sys_poll(struct pollfd __user *uf
> >  	fdt = files_fdtable(current->files);
> >  	max_fdset = fdt->max_fdset;
> >  	rcu_read_unlock();
> > -	if (nfds > max_fdset && nfds > OPEN_MAX)
> > +	if (nfds > max_fdset)
> >  		return -EINVAL;
> >
> >  	poll_initwait(&table);
>
> http://www.opengroup.org/onlinepubs/009695399/functions/poll.html sayeth
>
> [EINVAL]
>     The nfds argument is greater than {OPEN_MAX}, or ...
>
> and afaict, max_fdset can be either less than or greater than OPEN_MAX,
> depending upon how one sets NR_OPEN.

Very good point. My manpages have led me astray to the dark side. :)

> So I think that patch should be s/&&/||/ and s/changelog/new one/

Yikes, I think sticking the OR operator in there would seriously suck,
given that programs would then be unable to poll on more than 256 file
descriptors (which actually happens, or so I've heard). Besides, it
would also kill Tigran's work, as per the comment from the top of
select.c:

012  *  24 January 2000
013  *     Changed sys_poll()/do_poll() to use PAGE_SIZE chunk-based allocation
014  *     of fds to overcome nfds < 16390 descriptors limit (Tigran Aivazian).
015  */

> If anything we do here alters userspace-visible behaviour (and it probably will)
> then it should be *exahustively* documented in the changelog, and probably dropped.

Arguments of the cited standard's sensibilities notwithstanding (given
that it seems silly to limit the API in unnatural ways, as we can easily
and provably handle more than 256 fds), it seems better to leave this as
is. Thanks for the critical eye. :)

- Vadim Lobanov
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/