From: Andrew Morton <akpm@osdl.org>
To: Vadim Lobanov <vlobanov@speakeasy.net>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Fix poll() nfds check.
Date: Wed, 5 Jul 2006 20:39:59 -0700 [thread overview]
Message-ID: <20060705203959.53e128ef.akpm@osdl.org> (raw)
In-Reply-To: <Pine.LNX.4.58.0607051949460.6604@shell3.speakeasy.net>
On Wed, 5 Jul 2006 20:00:34 -0700 (PDT)
Vadim Lobanov <vlobanov@speakeasy.net> wrote:
> Hi,
>
> This is a trivial patch to fix the nfds check in the poll system call
> implementation. Namely, OPEN_MAX no longer does anything important in
> the kernel, and checking that nfds is greater than max_fdset AND greater
> than OPEN_MAX therefore just seems wrong.
>
> This brings up a slightly-tangential question: Why do the nfds checks
> exist in select()/poll()? They're not strictly necessary, since bad
> input will be caught later when we validate all the fds, one by one.
> Furthermore, these checks optimize the handling of error cases (which
> should be uncommon) while pessimizing correct usage of the syscalls
> (which should be more common).
>
> Signed-off-by: Vadim Lobanov <vlobanov@speakeasy.net>
>
> 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.
So I think that patch should be s/&&/||/ and s/changelog/new one/
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.
next prev parent reply other threads:[~2006-07-06 3:40 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-07-06 3:00 [PATCH] Fix poll() nfds check Vadim Lobanov
2006-07-06 3:39 ` Andrew Morton [this message]
2006-07-06 4:02 ` Vadim Lobanov
2006-07-06 15:44 ` Ulrich Drepper
2006-07-06 16:28 ` Vadim Lobanov
2006-07-07 3:00 ` H. Peter Anvin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20060705203959.53e128ef.akpm@osdl.org \
--to=akpm@osdl.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vlobanov@speakeasy.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.