Skip to content

Switching from crypto/rand to math/rand to avoid blocking#2454

Merged
estesp merged 1 commit intocontainerd:masterfrom
hairyhenderson:use-math-rand-instead-of-crypto-rand
Jul 12, 2018
Merged

Switching from crypto/rand to math/rand to avoid blocking#2454
estesp merged 1 commit intocontainerd:masterfrom
hairyhenderson:use-math-rand-instead-of-crypto-rand

Conversation

@hairyhenderson
Copy link
Copy Markdown
Contributor

@hairyhenderson hairyhenderson commented Jul 12, 2018

Addresses at least part of #2451...

In early-boot situations on systems without HWRNGs, containerd will hang on start until enough entropy has been gathered for crypto/rand's Read to unblock. In these cases, math/rand's Read is a more ideal choice since it's non-blocking.

Note that this is a much bigger deal than it was before patches for CVE-2018-1108 landed in the Linux kernel.

Signed-off-by: Dave Henderson [email protected]

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 12, 2018

Codecov Report

Merging #2454 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2454   +/-   ##
=======================================
  Coverage   45.04%   45.04%           
=======================================
  Files          92       92           
  Lines        9424     9424           
=======================================
  Hits         4245     4245           
  Misses       4496     4496           
  Partials      683      683
Flag Coverage Δ
#linux 49.29% <ø> (ø) ⬆️
#windows 41.33% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b382b6f...9a97ab3. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Jul 12, 2018

LGTM

estesp
estesp approved these changes Jul 12, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit cb4bf20 into containerd:master Jul 12, 2018
@hairyhenderson hairyhenderson deleted the use-math-rand-instead-of-crypto-rand branch July 12, 2018 16:49
@hairyhenderson
Copy link
Copy Markdown
Contributor Author

hairyhenderson commented Jul 12, 2018

Thanks!

@hairyhenderson hairyhenderson mentioned this pull request Jul 12, 2018
Closed
@AkihiroSuda
Copy link
Copy Markdown
Member

AkihiroSuda commented Jul 24, 2018

Was this cherry-picked?

@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Jul 24, 2018

@AkihiroSuda not yet, this change shouldn't be cherry-picked alone. I haven't figured out what the best set of changes to backport is or whether we still want another change to do seeding in a package rather than every main. If you have an idea or just want to cherry-pick this one first, go ahead

@semcodech
Copy link
Copy Markdown

semcodech commented Aug 29, 2018

@dmcgowan Could you please elaborate which changes need to be backported when cherry-picking this PR into one of the 1.0.x releases? Thanks a lot in advance.

@justincormack justincormack mentioned this pull request Dec 17, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estesp estesp estesp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@hairyhenderson @codecov-io @crosbymichael @AkihiroSuda @dmcgowan @semcodech @estesp