Linux cryptographic layer development
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Sandy Harris <sandyinchina@gmail.com>
Cc: Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
	Stephan Mueller <smueller@chronox.de>, John Denker <jsd@av8n.com>,
	m@ib.tc
Subject: Re: Lockless /dev/random - Performance/Security/Stability improvement
Date: Mon, 16 Aug 2021 10:25:37 -0400	[thread overview]
Message-ID: <YRp1YbpFdNG0IJMI@mit.edu> (raw)
In-Reply-To: <CACXcFmmosvNMxwjOFZ9SDadqJE11w6Vva+i9AV1zYQxbwoB9sA@mail.gmail.com>

On Mon, Aug 16, 2021 at 06:59:50PM +0800, Sandy Harris wrote:
> I am by no means convinced that Mike's idea of a lockless driver is a
> good one. Without some locks, if two or more parts of the code write
> to the same data structure, then there's a danger one will overwrite
> the other's contribution & we'll lose entropy.
> 
> However, I cannot see why any data structure should be locked when it
> is only being read. There's no reason to care if others read it as
> well. If someone writes to it, then the result of reading becomes
> indeterminate. In most applications, that would be a very Bad Thing.
> In this contact, though, it is at worst harmless & possibly a Good
> Thing because it would make some attacks harder.
> 
> For example, looking at the 5.8.9 kernel Ubuntu gives me, I find this
> in xtract_buf()
> 
> /* Generate a hash across the pool, 16 words (512 bits) at a time */
>     spin_lock_irqsave(&r->lock, flags);
>     for (i = 0; i < r->poolinfo->poolwords; i += 16)
>         sha1_transform(hash.w, (__u8 *)(r->pool + i), workspace);
> 
>     /*
>      * We mix the hash back into the pool ...
>      */
>     __mix_pool_bytes(r, hash.w, sizeof(hash.w));
>     spin_unlock_irqrestore(&r->lock, flags);
> 
> The lock is held throughout the fairly expensive hash operation & I
> see no reason why it should be.

The reason why this is there is because otherwise, there can be two
processes both trying to extract entry from the pool, and getting the
same result, and returning the identical "randomness" to two different
userspace processes.  Which would be sad....  (unless you are a
nation-state attacker, I suppose :-)

Cheers,

					- Ted

  reply	other threads:[~2021-08-16 14:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-09  0:33 Lockless /dev/random - Performance/Security/Stability improvement Mike Brooks
2021-06-11  3:59 ` Sandy Harris
2021-06-11  5:59   ` Stephan Müller
2021-06-11  9:53     ` Stephan Mueller
2021-06-11  9:43   ` Sandy Harris
     [not found]     ` <CALFqKjTAHvORw_U3sGe0ZRvAH8kTVKCdgVKQu+SK6h=C7B-jbA@mail.gmail.com>
2021-06-27 16:35       ` Mike Brooks
2021-08-16 10:59     ` Sandy Harris
2021-08-16 14:25       ` Theodore Ts'o [this message]
2022-01-27  0:35         ` Sandy Harris

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=YRp1YbpFdNG0IJMI@mit.edu \
    --to=tytso@mit.edu \
    --cc=jsd@av8n.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=m@ib.tc \
    --cc=sandyinchina@gmail.com \
    --cc=smueller@chronox.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