mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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.

  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 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome