Re: [PATCH RFC v1] random: implement getrandom() in vDSO

["Jason A. Donenfeld" <Jason@zx2c4.com>]

On Wed, Aug 03, 2022 at 12:27:43AM +0200, Thomas Gleixner wrote:
> Jason!

Thomas!!

> Not at all. The concept of 'basically same semantics' is a delusion. It
> does not exist. Either it's the same or it's not.
> 
> I really want to see your reaction on a claim that some RNG
> implementation is basically the same as the existing one. I'm sure you
> buy that without complaints.

I mean there are some undeniable similarities here. We might be arguing
over semantics of semantics at this point, but... Barring the additional
`void *state` argument, they take the same inputs and produce the same
outputs. Then, most importantly, the way they each produce randomness is
the same, using the same algorithms and same lifetime rules. For all
measures that are meaningful to me, they're the "same". Yes yes
technically that extra `void *state` means something is slightly
different.  But does that matter? I guess your point is that somehow it
does matter, and maybe that's where we disagree.

> >> So you have to go through the whole process of a new ABI whether you
> >> like it or not.
> >
> > Ahh, in that sense. Yea, I'd rather not have to do that too, with the
> > additional opaque handle passed as the first argument. It'd be nice if
> > there were some private place where I could store the necessary state,
> > but I'm not really sure where that might be at the moment. If you have
> > any ideas, please let me know.
> 
> That's exactly the problem. VDSO is a stateless syscall wrapper which
> has to be self contained for obvious reasons.

Unless each call can import and export any potential state. That is,
unless there's a `void *state` argument like what I added. Then the code
itself doesn't refer to any global state, but can still behave in a
stateful manner.

We could even establish this as a "convention" if necessary, and
document it as such. That way upgrading a syscall ABI to a vDSO stateful
ABI always follows some rule, and then this doesn't feel as ad-hoc.
Heck, it could even be an opaque type, `vdso_handle_t state`, which
would just be a unsigned long.

> My previous statement:
> 
>     Everything else is library material, really.
> 
> is based on that fact and not on the unwillingness to add magic muck to
> the VDSO.
> 
> The unwillingness part is just the question:
> 
>     Is there a sensible usecase?

In the use case department, by the way, apparently there really is.
arc4random() is too slow for chronyd:
https://sourceware.org/bugzilla/show_bug.cgi?id=29437
And that's on a kernel even with the "newer faster getrandom()".

> Assumed that there is a sensible usecase, there is a way out and that's
> exactly the library part. You can make that VDSO interface versioned and
> provide a library in tools/random/ which goes in lockstep with the VDSO
> changes.

That sounds absolutely dreadful; no way jose. Then we wind up having to
maintain the data in the vDSO as a particular version, and keep that
working into the future. That's not going to fly. As I wrote before, I
don't want to commit the RNG to preserving certain internal semantics
over time. That's not something I feel comfortable committing to. And
even if we can "add" new ones with a new version in your hypothetical
scheme, we'd still have to keep the old ones working, and that could
prove prohibitive of improvements. So that's not going to work.

And again, I don't see how this is any different than gettimeofday() and
such. Why didn't you just make versioned accessor functions for each one
of the various struct fields, and then stick some library code into
tools/timekeeping/ for glibc to copy and paste once and never update
again? Why isn't this a nightmare you chose to have each time the moon
is full? Clearly because doing so would be a maintenance disaster and
would impede future meaningful progress, not to mention proliferating
wrong means of reading the time.

Just as gettimeofday benefits from being an actual function in the vDSO,
so too does getrandom().

> Vs. the storage problem. That yells TLS, but that makes your process
> wide sharing moot, which might not be the worst of all things IMO.

Yea, TLS is what we want here. The `void *state` argument thing is meant
for this. You allocate an array of states using that alloc function, and
then you divvy them up per-thread.

Jason