Re: [PATCH v6 3/3] KVM: selftests: randomize page access order

[Sean Christopherson <seanjc@google.com>]

On Tue, Oct 11, 2022, Colton Lewis wrote:
> Sean Christopherson <seanjc@google.com> writes:
> 
> > On Tue, Oct 11, 2022, Colton Lewis wrote:
> > > Sean Christopherson <seanjc@google.com> writes:
> > > > Ya, I'm trippling (quadrupling?) down on my suggestion to improve the
> > > > APIs.  Users
> > > > should not be able to screw up like this, i.e. shouldn't need comments
> > > > to warn
> > > > readers, and adding another call to get a random number shouldn't
> > > affect
> > > > unrelated
> > > > code.
> 
> > > Previous calls to PRNGs always affect later calls to PRNGs. That's how
> > > they work.
> 
> > Ya, that's not the type of bugs I'm worried about.
> 
> > > This "screw up" would be equally likely with any API because the caller
> > > always needs to decide if they should reuse the same random number
> > > or need a
> > > new one.
> 
> > I disagree, the in/out parameter _requires_ the calling code to store
> > the random
> > number in a variable.  Returning the random number allows consuming the
> > number
> > without needing an intermediate variable, e.g.
> 
> > 	if (random_bool())
> > 		<do stuff>
> 
> > which makes it easy to avoid an entire class of bugs.
> 
> Yes, but it's impossible to do this without hidden global state at the
> implementation level. That sacrifices reentrancy and thread-safety.

The above is super quick pseudocode that wasn't intended to be taken verbatim.
From my original suggestion in patch one[*], throw the seed/metadata in a opaque
struct, e.g. ksft_pseudo_rng (or kvm_pseudo_rng if the code ends up being KVM-only).

The intent is to hide the details of the rng, both so that the caller doesn't have
to worry about those details, but also so that the guts can be changed at will
without having to update callers.

[*] https://lore.kernel.org/all/20220912195849.3989707-2-coltonlewis@google.com