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.

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.

Oops. Use C89-style comments per style(9).

Add comment explaining the cast.

In D31327#705944, @bdrewery wrote:

I don't know exactly but I figured it was simple latency with something like debugnet_arp_gw sending arp request 1, delay(500), retry -> arp request 2 sent, arp reply 1 received (state changes), delay(500), arp reply 2 received (panic due to unexpected state).

This would only happen if you have multiple competing dhcp servers on a subnet, right? Edit: not dhcp, oops. Still, kind of surprising we’re getting multiple ARP responses?

Use ssize_t.

In D31292#704741, @kib wrote:

ptrdiff_t sounds arbitrary. Why did not you used e.g. ssize_t? Because it is not defined by C STD?

Is there a way we could make MB_CUR_MAX or __mb_cur_max suck less, in a more general way? Like, isn't it basically always four or one?

I think the idea is very reasonable and could be done in a way that preserves the safety of the sandbox, at least for fstyp. I don't think the popen approach makes sense. There may be a lot of work required to sandbox the suggested consumer (mount).

Nice cleanup. I didn't review the man page language updates super thoroughly, but the parts I skimmed looked good.

Merci

Thanks!

Nice find. I'd encourage using prng32_bounded instead, but I don't think this is mechanically wrong.

Awesome, thanks!

Seems reasonable to me!

Generally seems good to me.

Nah

In D29007#649973, @jmg wrote:

In D29007#649700, @cem wrote:

Is 128 a reasonable chunk here? How does it relate to the internal buffering of FILE?

I'm going to run some perf numbers on ARM, but w/ the attached file w/ benchmarks, it looks like the chunk should be closer to 16.

Certainly not a regression :-)

Is 128 a reasonable chunk here? How does it relate to the internal buffering of FILE?

Two small issues remaining, but otherwise I think it's in good shape

In D12330#648476, @missoline_protonmail.com wrote:

@cem Hi there! Is there a link to the v2 ?

Still accepted from earlier

Outside of ossl_chacha20.c, everything looks fine.

Hm, it is little endian, but I'm not confident about the two sentences prior.

The analysis makes sense and the fix looks fine to me. You could allocate two fds and check socketpair against both (instead of just low) in the final test, but I don't think there's much marginal value in that. (Frankly, I'm not a fan of unix' must-assign-lowest-fd behavior in general, but we have to abide by it.)

In the commit summary, is this a typo?

No objection in principle, but the diff is basically unreviewable as-is for me; it's like 90% autoconf generated shell.

LGTM, thanks.

Looks great to me!

In D28395#636443, @arichardson wrote:

Turns out the bug was that I messed up the size being passed to the SSE4.2 version. It does work correctly.

In D28375#634909, @cem wrote:

incore has always been able to produce false negatives, FWIW. It's fundamentally racy. I think it's mostly used as a heuristic for avoiding unnecessary prefetch.

That's true, but it's possible for callers to depend on the instantaneous result being correct because some external synchronization guarantees that the result is stable. For instance, some lock might serialize calls to getblk() for a particular vnode, so holding that lock ensures that incore() returns a stable result. I'm worried about false negatives caused by reassignbuf() in cases like this.

In D28395#635155, @arichardson wrote:

I think QEMU provides a basic amd64 CPU by default which does not include any newer features (wikipedia suggests SSE4.2 was introduced ~2008).

I think the idea here is that CRC32 is part of SSE4.2, which is part of the base feature set of amd64. If you're hitting SIGILL in QEMU in amd64 mode, that suggests QEMU's amd64 emulation is somewhat invalid. However, it's certainly optional on at least some early models of i386.