Is the associated hardware documentation public, and if so, could you link to it?

I have no objection.

Yes, yes. It is easier to analyze with e.g. rand2() % 3. But you and Ed have both mentioned a way to measure this for rand32 more tractably -- great!

Yeah, you can do a simple bias test with an upper limit of 3 for example. Bucket 0 is more likely than buckets 1 and 2 under a simplistic rand32() % 3 implementation. Or something like that. It is probably easier to measure with a smaller power of 2 generator.

Some context on the Lemire idea if others haven't seen it:

Thanks.

This is now sort of a layering violation, right? We're assuming Fortuna implements devrandom, but the interface is pluggable. We have 2-3 implementations in tree, and integrators might write their own for compliance reasons.

LGTM.

In random_early_prime we divide the input into blocks of size sizeof(event.he_entropy)) and then process those one by one, with each block only being fed into one pool. so simply padding the entropy with zeros would result in most of the pools having no entropy.

I don't think there's any reason to spread this using pbkdf2 just to defeat the Fortuna input seeding thresholds. You might as well seed the 64 bytes and then 2048-64 bytes of zero, or whatever. (Internally Fortuna is compressing the provided seed material using SHA2.)

Lgtm

Nice observation and fix.

No objection. I'll let someone else stamp it.

Lgtm.

Nevermind

LGTM

As mentioned in email, we might also consider publishing a seed generation to the VDSO page from Fortuna.

Something to keep in mind re: exactly 64 is that it's possible for entropy sources to return less entropy than was requested, if for example some internal state means they don't have anything available at the moment. That's one reason I prefer polling repeatedly until seeded (or some spin threshold is crossed).

Discussed with: cem

Thanks.

Seems reasonable to me.

Seems very reasonable to me. Thanks!

Seems reasonable to me!

LGTM. My only bikeshed contribution is maybe gctl_get_devnameparam or get_devparam. But I don’t object to the current name.

Thanks.

Setting cstd seems strongly preferable vs making upstream updates more difficult.

Either way seems fine to me. I’d ask vangyzen or bdrewery for input.

What workflow?

In D34220#774261, @sg2342_googlemail.com wrote:

subtract one from vt_logo_sprite_height as well

In D34186#773413, @dim wrote:

In D34186#773402, @cem wrote:

Looks like TESTAB expects boolean (0 and 1) values, too.

Well, the interesting thing is that this macro produces values 0 through 4, and it's a sort of weird shortcut to make the switch(TESTAB()) construct possible.

Looks like TESTAB expects boolean (0 and 1) values, too.

In D34152#772589, @hselasky wrote:

@cem: You mean "iff" ? if and only if?

Looks fine to me. We should probably also actively feed jitter entropy if we hit the blocked state on boot, but that doesn't mean we shouldn't just do this first.

Typo in summary: s/path/patch/

Thanks.

I’m hoping emaste will be able to test and commit it.

LGTM, thanks.

LGTM modulo overflow concern and what looks like a typo.

Lgtm. I didn’t attempt to verify the asm implementation.

Seems ok in principle.

At a minimum, you have rdseed (rdrand). But I expect there are other non-ethernet sources present as well.

The problem is that in some use cases we might not have a lot of entropy good sources, with the ethernet being the only good candidate.

Consider just disabling Ethernet entropy collection instead. In fact, I thought it was off by default in approximately the 13 timeframe. Maybe even 12.

Cool

Fortuna doesn't specify the 100ms sleep behavior, as far as I can tell. Removing it seems reasonable to me.

This change should come with a motivational ministat graph.

I meant fundamentally bad idea. Very little work should happen in interrupt context. Taking a global lock and running AES in software is somewhat expensive.

I don't believe anything should be consuming random in interrupt contexts. Could you elaborate on the scenario / bug?

Address of packed member? How?

Lgtm, thanks for driving this.

Let’s re-remove read_rate_increment but otherwise it’s looking good to me.

In D32013#722497, @kevans wrote:

OK, I can throw together a patch to this effect. Based on your description here, we're tentatively punting on the idea of just collecting into the zero pool for now, right? My understanding is that 128 bits into the zero pool is the bare minimum and 256 bits per pool per second as a good compromise between there and where we're at now.

In D32013#722391, @kevans wrote:

If that's an option, then sure. =-) I didn't/don't understand fortuna's design well enough to make claims about what a reasonable rate is. If we can securely drop it down to <= 1kB/s on average then yeah, we can probably just not care about this at all.

I hadn’t seen the patch moving the stack buffer to the softc when I wrote my earlier remarks. I still think we should be polling less volume.

As mentioned earlier in the stack I don’t think this mitigation is great.

I don’t think this is a great mitigation for random - the pending request will still be written in guest memory, and we need the queue completion to know when we can free the memory.

Lgtm.

LTO builds can see across CUs. I don’t know of any particular pass that would eliminate this, though.

In D31510#710608, @markj wrote:

What exactly is the UB here? I'm not sure how the compiler could legally eliminate SHA256_Update(&ctx, key, sizeof(key)).

It's not exactly a false positive, although poisoning the output as uninitialized is sort of unhelpful. Are we confident the compiler isn't eliminating access to it (due to UB) outside of KMSAN? In the abstract, I think we would prefer to eliminate bypass_before_seeding.