public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Michael Kerrisk <mtk.manpages@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] urandom: handle signals immediately
Date: Fri, 19 Dec 2014 11:57:39 -0500	[thread overview]
Message-ID: <20141219165739.GA11655@thunk.org> (raw)
In-Reply-To: <1417252349-808-1-git-send-email-xypron.glpk@gmx.de>

On Sat, Nov 29, 2014 at 10:12:29AM +0100, Heinrich Schuchardt wrote:
> Without the patch device /dev/urandom only considers signals when a
> rescheduling of the thread is requested. This may imply that
> signals will not be handled for time intervals in excess of 30s.

Sorry, I didn't see your e-mail for a while; it got lost in my inbox
due to my being travelling for Thanksgiving weeksend.

I'm not sure where you are getting 30 seconds from, but you're right
that it would be better to check signal_pending() on each loop.  That
being said, your patch isn't right.

> +		/*
> +		 * getrandom must not be interrupted by a signal while
> +		 * reading up to 256 bytes.
> +		 */
> +		if (signal_pending(current) && ret > 256)
> +			break;
> +		if (need_resched())
>  			schedule();
> -		}

This means that we can reschedule even for small requests, and that's
no good; getrandom *must* be atomic.  You also need to return
-ERESTARTSYS if we get interrupted with no bytes.  So this needs to be
something like this:

		if (ret > 256) {
			if (signal_pending(current)) {
				if (ret == 0)
					ret = -ERESTARTSYS;
				break;
			}
			if (need_resched())
				schedule();
		}

Cheers,

					- Ted

  reply	other threads:[~2014-12-19 16:57 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1412295197-8100-1-git-send-email-xypron.glpk@gmx.de>
     [not found] ` <1412295313-8198-1-git-send-email-xypron.glpk@gmx.de>
2014-10-28 11:37   ` [PATCH 1/3] getrandom.2: new manpage Michael Kerrisk (man-pages)
2014-11-11 11:44   ` Michael Kerrisk (man-pages)
2014-11-11 16:19     ` [PATCH] getrandom.2: treatment of interrupts Heinrich Schuchardt
2014-11-16 15:55       ` Michael Kerrisk (man-pages)
2014-11-22 11:28         ` Heinrich Schuchardt
2014-11-29  9:12           ` [PATCH 1/1] urandom: handle signals immediately Heinrich Schuchardt
2014-12-19 16:57             ` Theodore Ts'o [this message]
2014-12-19 18:55               ` Heinrich Schuchardt
2015-01-10 13:23     ` [PATCH 1/3] getrandom.2: new manpage Michael Kerrisk (man-pages)

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=20141219165739.GA11655@thunk.org \
    --to=tytso@mit.edu \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtk.manpages@gmail.com \
    --cc=xypron.glpk@gmx.de \
    /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