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@cs-soprasteria.com 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