LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: nhorman@tuxdriver.com
Cc: linux-kernel@vger.kernel.org, akpm@osdl.com, torvalds@osdl.com
Subject: Re: [PATCH] select: fix sys_select to not leak ERESTARTNOHAND to userspace
Date: Tue, 23 Jan 2007 21:59:26 -0800 (PST)	[thread overview]
Message-ID: <20070123.215926.85409740.davem@davemloft.net> (raw)
In-Reply-To: <20070116201332.GA28523@hmsreliant.homelinux.net>

From: Neil Horman <nhorman@tuxdriver.com>
Date: Tue, 16 Jan 2007 15:13:32 -0500

> As it is currently written, sys_select checks its return code to convert
> ERESTARTNOHAND to EINTR.  However, the check is within an if (tvp) clause, and
> so if select is called from userspace with a NULL timeval, then it is possible
> for the ERESTARTNOHAND errno to leak into userspace, which is incorrect.  This
> patch moves that check outside of the conditional, and prevents the errno leak.
> 
> Signed-Off-By: Neil Horman <nhorman@tuxdriver.com>

As has been probably mentioned, this change is bogus.

core_sys_select() returns -ERESTARTNOHAND in exactly the
case where that return value is legal, when a signal is
pending, so that the signal dispatch code (on return from
the system call) will "fixup" the -ERESTARTNOHAND return
value so that userspace does not see it.

What could be happening is that, somehow, we see the signal
pending in core_sys_select(), but for some reason by the time
the signal dispatch code would run, the signal is cleared.

I always found this scheme a little fishy, relying on signal
pending state to guarentee the fixup of the syscall restart
error return values.

For example, I see nothing that prevents another thread sharing
this signal state from clearing the signal between the syscall
checking in the other thread and the signal dispatch check running
in that other thread.

	cpu 1			cpu 2
	check sigpending
				clear signal
	syscall return
	no signal panding
	get -ERESTARTNOHAND

Perhaps something makes this no happen, but I don't see it :)

  parent reply	other threads:[~2007-01-24  5:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-16 20:13 Neil Horman
2007-01-22 13:59 ` Paolo Ornati
2007-01-22 14:52   ` Neil Horman
2007-01-22 16:03     ` Linus Torvalds
2007-01-22 16:24       ` Neil Horman
2007-01-23  0:00         ` bert hubert
2007-01-24  5:59 ` David Miller [this message]
2007-01-24 13:21   ` Neil Horman
2007-08-17 15:41 John Blackwood
2007-08-17 20:55 ` Neil Horman

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=20070123.215926.85409740.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=akpm@osdl.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=torvalds@osdl.com \
    --subject='Re: [PATCH] select: fix sys_select to not leak ERESTARTNOHAND to userspace' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).