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

random generator update for Mac proposal #4081

Merged
merged 1 commit into from Jan 17, 2021

Conversation

devnexen
Copy link
Contributor

No description provided.

Copy link
Member

@shyouhei shyouhei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please provide a bit more detailed descriptions.

aclocal.m4 Outdated
@@ -43,6 +43,5 @@ m4_include([tool/m4/ruby_stack_grow_direction.m4])
m4_include([tool/m4/ruby_try_cflags.m4])
m4_include([tool/m4/ruby_try_cxxflags.m4])
m4_include([tool/m4/ruby_try_ldflags.m4])
m4_include([tool/m4/ruby_type_attribute.m4])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tell us the reason for this removal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected change, I ll roll it back.

# if defined(__ANDROID__) || defined(__APPLE__)
# include <sys/random.h>
# endif
# define MAX_SEED_LEN_PER_READ 256
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the reason behind this 256?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It s to avoid EIO errno case since getentropy can handle 256 bytes maximum per call.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The length is described in the man-pages, but no symbol seems defined for it.

macOS:

The maximum buffer size permitted is 256 bytes.  If buflen exceeds this, an error of EIO will be indicated.

Linux:

The maximum permitted value for the length argument is 256.

random.c Outdated
Comment on lines 425 to 427
# if defined(__ANDROID__) || defined(__APPLE__)
# include <sys/random.h>
# endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is the code for <sys/random.h> already, this should be there too.
That is, at line 52,

#if defined HAVE_GETRANDOM || defined HAVE_GETENTROPY

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked if I remove this line it fails to compile

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It means to add || defined HAVE_GETENTROPY to line 52.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah gotcha sorry.

random.c Outdated
@@ -497,7 +516,8 @@ fill_random_bytes_syscall(void *buf, size_t size, int unused)
{
#if (defined(__OpenBSD__) && OpenBSD >= 201411) || \
(defined(__NetBSD__) && __NetBSD_Version__ >= 700000000) || \
(defined(__FreeBSD__) && __FreeBSD_version >= 1200079)
(defined(__FreeBSD__) && __FreeBSD_version >= 1200079) || \
defined(__APPLE__)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what cases is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not ... too much randomness at a time in this case indeed :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this condition is enabled, this fill_random_bytes_syscall() never fails and fill_random_bytes_urandom() using getentropy() is never used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true I removed it in the last commit anyway.

using getentropy for seeding, reading 256 bytes at a time to avoid
 the EIO errno since this is the maximum.
@nobu nobu merged commit 54c9118 into ruby:master Jan 17, 2021
@mame
Copy link
Member

mame commented Jan 18, 2021

@mame
Copy link
Member

mame commented Jan 18, 2021

6abf393

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants