linux-kernel - Re: [PATCH 1/1] urandom: handle signals immediately

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141219165739.GA11655@thunk.org>
Date:	Fri, 19 Dec 2014 11:57:39 -0500
From:	Theodore Ts'o <tytso@....edu>
To:	Heinrich Schuchardt <xypron.glpk@....de>
Cc:	Arnd Bergmann <arnd@...db.de>,
	Michael Kerrisk <mtk.manpages@...il.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] urandom: handle signals immediately

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/