Le 28/08/2024 à 16:49, Jason A. Donenfeld a écrit :
Hi Christophe,
On Wed, Aug 28, 2024 at 4:38 PM LEROY Christophe <christophe.leroy2@xxxxxxxxxxxxxxxxxx> wrote:
- key[0], key[1], key[2], key[3], key[4], key[5], key[6], key[7]
+ le32toh(key[0]), le32toh(key[1]), le32toh(key[2]), le32toh(key[3]),
+ le32toh(key[4]), le32toh(key[5]), le32toh(key[6]), le32toh(key[7])
Are you sure about that?
I'm sure it is needed to get the same behaviour as before.
So I don't see any endianness conversion happening with the key bytes.
They're memcpy'd from rng output bytes directly into native endian u32
words.
You may have an objection to this. But the goal of the vDSO code is to
match the kernel's algorithm 1:1 without deviations. To that end, I
suspect this patch actually improves the unit test to ensure that.
With regards to your objection, though, if you feel strongly enough
about it, I suppose that's something you could attempt to change
throughout, with one commit that touches random.c and the vDSO code. I'm
not sure whether or not I'd go along with that yet, but if it were to
happen, I think that's the way to do it. For now, though, the goal is
for the vDSO algorithm to copy the kernel algorithm.
Do you agree that this patch helps the vDSO algorithm copy the kernel
algorithm better? Genuinely asking, because maybe I got it all backwards
somehow.
As I said several times, I was a bit puzzled by the fact that I had to
read key words in reversed byte order when I implemented the powerpc
chacha20. This was needed to get the selftest succeed. So if doing it
the new way makes it closer to the kernel implementation, I'm 100% fine
with it, it will also slightly simplify powerpc chacha20 vDSO function.
The only thing is that you must describe this behaviour change in your
commit message, you can't just let people believe it is a one-to-one
replacement of the previous test implementation that used Sodium.
Christophe
