Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

data races in mirage-crypto #220

Closed
2 tasks done
hannesm opened this issue Mar 19, 2024 · 9 comments
Closed
2 tasks done

data races in mirage-crypto #220

hannesm opened this issue Mar 19, 2024 · 9 comments

Comments

@hannesm
Copy link
Member

hannesm commented Mar 19, 2024

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

@hannesm hannesm changed the title data races in miarge-crypto data races in mirage-crypto Mar 19, 2024
@palainp
Copy link
Member

palainp commented Mar 25, 2024

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
Copy link
Member

#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
Copy link
Member

palainp commented Mar 25, 2024

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
Copy link
Member

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
Copy link
Member Author

hannesm commented Mar 26, 2024

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
Copy link
Member Author

hannesm commented Mar 27, 2024

See #223 for DES without global state

@hannesm
Copy link
Member Author

hannesm commented Mar 27, 2024

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
Copy link
Member Author

hannesm commented Mar 27, 2024

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
Copy link
Member

palainp commented Mar 27, 2024

Thanks for the cherry picking work!

@hannesm hannesm closed this as completed Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants