Re: [PATCH v14 7/7] x86: vdso: Wire up getrandom() vDSO implementation

[Jason A. Donenfeld]

Hi Christophe,

Thanks for the review.

On Thu, Jan 12, 2023 at 6:27 PM Christophe Leroy
<christophe.leroy@xxxxxxxxxx> wrote:
> Le 01/01/2023 à 17:29, Jason A. Donenfeld a écrit :
> > Hook up the generic vDSO implementation to the x86 vDSO data page. Since
> > the existing vDSO infrastructure is heavily based on the timekeeping
> > functionality, which works over arrays of bases, a new macro is
> > introduced for vvars that are not arrays.
> >
> > The vDSO function requires a ChaCha20 implementation that does not write
> > to the stack, yet can still do an entire ChaCha20 permutation, so
> > provide this using SSE2, since this is userland code that must work on
> > all x86-64 processors. There's a simple test for this code as well.
>
> As far as I understand the test is not dependent on the architecture,
> can it be a separate patch ?

The test is dependent on architectures for which there's a vDSO
implementation. I could move it to a patch before or after this one,
though, if you think it'd be better to keep this commit as a template
commit for other architectures.

> Also, as the chacha implementation is standalone and can be tested by
> the above mentionned simple test, can it be a separate patch as well ?

No, that actually needs to be part of this one, as it's part and
parcel of the x86 implementation.

> Then the last patch only has the glue to wire-up getrandom VDSO to the
> architecture, and can be used as a reference for other architectures.

This is part of the required glue, so no, it belongs in this one.
> > + *   rdx: 8-byte counter input/output
>
> Why a 8-byte counter ? According to RFC 7539, chacha20 takes:
> Are you mixing up the upper part of the counter with the nonce ? In that
> case you can't say you use a 0 nonce, can you ?

No, I'm not mixing anything up. This is the same algorithm that
random.c uses. And wireguard, for that matter. 8 byte nonce, 8 byte
block counter. In this case, the nonce is 0.

> > +#include <sodium/crypto_stream_chacha20.h>
>
> Is that standard ? Every distribution has sodium ?

As far as I can tell, yes.

Jason