Hi Eric,
On Sat, Feb 5, 2022 at 8:00 AM Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
> I tested this, and it actually was 205 calls prior to patch 1 in this series,
> and 267 calls after patch 1. That's in contrast to 256 calls after this patch.
> Not a big difference, but this is going to result in ~25% more single-bit calls
> being needed compared to the old version. It's unclear whether you're arguing
> that's basically the same, or whether you thought it was a smaller difference.
My argument is that we're not _decreasing_ the security in any
substantive way with this change.
> Doesn't the default value of random_write_wakeup_bits need to be increased to
> this value? Otherwise, the pool can get stuck with entropy_count greater than
> or equal to random_write_wakeup_bits (192) but less than POOL_MIN_BITS (256).
Good catch, thanks.
> In fact, the only correct value of random_write_wakeup_bits will be 256, i.e.
> the entire pool. Perhaps it should no longer be configurable via /proc/sys?
I think so, yea. I'll change up in an add-on commit.
> Note that there's also an off-by one bug that will need to be fixed:
> add_hwgenerator_randomness() checks entropy_count <= random_write_wakeup_bits
> rather than entropy_count < random_write_wakeup_bits as random_poll() does.
Thanks.
> > + do {
> > + entropy_count = orig = READ_ONCE(input_pool.entropy_count);
> > + entropy_count = min(POOL_BITS, entropy_count + nbits);
> > + } while (cmpxchg(&input_pool.entropy_count, orig, entropy_count) != orig);
>
> This can be simplified slightly:
>
> do {
> orig = READ_ONCE(input_pool.entropy_count);
> entropy_count = min(POOL_BITS, orig + nbits);
> } while (cmpxchg(&input_pool.entropy_count, orig, entropy_count) != orig);
That looks nicer indeed. Will do.
Thanks for your comments.
Jason
