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

Use libc::SYS_getrandom #839

Closed
wants to merge 1 commit into from
Closed

Conversation

josephlr
Copy link
Contributor

@josephlr josephlr commented Jun 13, 2019

This has been in libc for a while, so we don't need to increment the
libc version in Cargo.toml. Note that we cannot use the getrandom()
function from libc, as it isn't available on Android.

Originally part of #744

@briansmith
Copy link
Owner

Can we just fix rust-lang/libc#659 (comment) and then use libc::getrandom() instead? I'd like to remove the direct use of syscall by ring completely.

@josephlr
Copy link
Contributor Author

Can we just fix rust-lang/libc#659 (comment) and then use libc::getrandom() instead? I'd like to remove the direct use of syscall by ring completely.

One particular issue is that getrandom is only supported on fairly new glibc/musl versions (despite the syscall being around for awhile), and isn't supported on Android. If you're OK with:

  • Requiring a recent glibc/musl
  • Potentially going back to using the raw syscall if ring ever supports Android

Then I think going with rust-lang/libc#1399 is the best bet.

@josephlr
Copy link
Contributor Author

josephlr commented Jun 14, 2019

Potentially going back to using the raw syscall if ring ever supports Android

@briansmith disregard the above, I forgot that ring currently supports Android. So it looks like using libc::getrandom is not an option, so we will have to stick with the raw syscall (i.e. this PR). 🙁

Edit: For context see rust-lang/libc#1399 (comment) and boostorg/uuid#76

This has been in libc for a while, so we don't need to increment the
libc version in Cargo.toml. Note that we cannot use the `getrandom()`
function from libc, as it isn't avalible on Android.
@josephlr
Copy link
Contributor Author

@briansmith this is ready to merge. rust-lang/libc#1399 was merged, but we can't use it because the bindings are only available for Linux, not Android.

@briansmith
Copy link
Owner

What's the specific goal that you are trying to achieve here? Is there something wrong with the constants that ring is currently using? Are you hoping to do another Linux port that isn't supported by ring yet? Or something else?

@josephlr
Copy link
Contributor Author

What's the specific goal that you are trying to achieve here?

@josephlr
Copy link
Contributor Author

Closing for similar reasons to #856, this is trying to bring over chunks from the getrandom create which is the wrong approch.

@josephlr josephlr closed this Jul 12, 2019
@josephlr josephlr deleted the getrandom branch July 12, 2019 23:14
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

Successfully merging this pull request may close these issues.

2 participants