data races in mirage-crypto

[hannesm]

At the time being, there are two things to consider:

• [x] C code for DES contains global mutable data (in the key derivation) <- this is not data race free #223
• [x] RNG contains two mutable values: entropy sources and default generator <- here some Atomic compare-and-swap or similar should be done to avoid races (see #186 for inspiration) #221

[palainp]

Hi, I'm not really aware about domains and multicore stuffs in Ocaml5, so I may be wrong here, but for the DES item, couldn't using the __thread qualifier be ok? (I tried https://github.com/palainp/mirage-crypto/tree/des-thread and I can see that the .tbss section is now present in bench/speed.exe and, as we now we have .tbss and .tdata with solo5, it should works).

[dinosaure]

221 is a proposal for RNG and be domain safe. Also, I like the @palainp remarks, we probably should test it (eg. run in parallel encrypt/decrypt)

[palainp]

I'm not sure storing the keys in thread-local storage is a good path, as keys from different domains won't be shared :( Maybe using a mutex approach would be better?

[dinosaure]

I'm not sure storing the keys in thread-local storage is a good path, as keys from different domains won't be shared :( Maybe using a mutex approach would be better?

Using TLS was the first idea I had for continuing to use global buffers but not sharing them between domains. Currently the use of these global buffers is correct, even taking ocaml-tls into account, and it is difficult to imagine encrypting/decrypting a source across several domains (on the other hand, this can be done concurrently, as with lwt).

Using a mutex would have a very significant impact on performance.

[hannesm]

uhm, hold on... maybe we can find some DES code that doesn't use global data (and has a public domain license)? Or modify the code that is part of mirage-crypto to get the key structures passed explicitly?

[hannesm]

See #223 for DES without global state

[hannesm]

Hi, I'm not really aware about domains and multicore stuffs in Ocaml5, so I may be wrong here, but for the DES item, couldn't using the __thread qualifier be ok? (I tried https://github.com/palainp/mirage-crypto/tree/des-thread and I can see that the .tbss section is now present in bench/speed.exe and, as we now we have .tbss and .tdata with solo5, it should works).

your const stuff would be very welcome on top of #223 :)

[hannesm]

Hi, I'm not really aware about domains and multicore stuffs in Ocaml5, so I may be wrong here, but for the DES item, couldn't using the __thread qualifier be ok? (I tried https://github.com/palainp/mirage-crypto/tree/des-thread and I can see that the .tbss section is now present in bench/speed.exe and, as we now we have .tbss and .tdata with solo5, it should works).

your const stuff would be very welcome on top of #223 :)

nevermind, I'll pick those up and push to 223 :)

[palainp]

Thanks for the cherry picking work!