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

runtime: use getrandom(2) for readRandom #68278

Open
apocelipes opened this issue Jul 2, 2024 · 13 comments · May be fixed by #69421
Open

runtime: use getrandom(2) for readRandom #68278

apocelipes opened this issue Jul 2, 2024 · 13 comments · May be fixed by #69421
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@apocelipes
Copy link
Contributor

apocelipes commented Jul 2, 2024

Proposal Details

Currently when we set the random seed on Linux, the runtime's fallback path needs to open, read and then close "/dev/urandom". This requires 3 system calls. A single getrandom does the same thing and is simpler than using open() -> read() -> closefd().

#67001 Raised the minimum supported Linux kernel version to 3.17+, so that "getrandom(2)" can be used in the runtime package and we can do all the things in one system call now.

We can wrap getrandom with internal/runtime/syscall.Syscall6 so that the helper function readRandom will no longer need to open and close "/dev/urandom".

Since Go 1.24 will only support Linux kernel 3.17+ (or 3.10+ with getrandom patches), I think a fallback is unnecessary (and a fallback can easily be added if someone needs it).

@gopherbot gopherbot added this to the Proposal milestone Jul 2, 2024
@gabyhelp
Copy link

gabyhelp commented Jul 2, 2024

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@seankhliao seankhliao changed the title proposal: runtime: use getrandom(2) to set random seed on Linux runtime: use getrandom(2) for readRandom Jul 2, 2024
@seankhliao seankhliao removed this from the Proposal milestone Jul 2, 2024
@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. compiler/runtime Issues related to the Go compiler and/or runtime. labels Jul 2, 2024
@seankhliao
Copy link
Member

I don't think a proposal is necessary as there are no user facing changes?
Crashing on unavailability seems ok given #66821

@randall77
Copy link
Contributor

On linux at least, the runtime does not normally call getrandom. We use the auxv random data at startup.
The readRandom path is kind of a fallback.
(I'm not sure precisely when linux gives us the auxv. But I imagine most modern linuxes do - that path has been in the codebase for a while.)

@apocelipes
Copy link
Contributor Author

On linux at least, the runtime does not normally call getrandom. We use the auxv random data at startup. The readRandom path is kind of a fallback. (I'm not sure precisely when linux gives us the auxv. But I imagine most modern linuxes do - that path has been in the codebase for a while.)

Yes, but I think it's also worth to simplify the fallback path. All this changes will be at most a middle size CL.

@randall77
Copy link
Contributor

Yeah, no problem, I was not arguing against the proposed change. Simplification is good. Just mentioning it probably won't affect any behavior.

@randall77
Copy link
Contributor

Given #67001 is accepted, I don't think this needs to be a proposal. Taking out of the proposal process.

@randall77 randall77 added this to the Backlog milestone Jul 2, 2024
@randall77 randall77 added help wanted NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 2, 2024
@apocelipes
Copy link
Contributor Author

Given #67001 is accepted, I don't think this needs to be a proposal. Taking out of the proposal process.

Thanks. I'd like to send a CL to implement this when the next development window opens.

@Jorropo
Copy link
Member

Jorropo commented Jul 3, 2024

I think this is a duplicate part of #66821, all of the content of this issue seems to be covered by #66821 already.

@randall77
Copy link
Contributor

I think #66821 is about crypto/rand, not the runtime's internal randomness.
There's also math/rand and math/rand/v2, not sure if those interact with a source of randomness or not.

@mauri870
Copy link
Member

mauri870 commented Jul 3, 2024

Looks like math/rand/v2 uses runtime.rand for the random source of top level functions

go/src/math/rand/v2/rand.go

Lines 254 to 259 in 71f9dbb

// globalRand is the source of random numbers for the top-level
// convenience functions.
var globalRand = &Rand{src: runtimeSource{}}
//go:linkname runtime_rand runtime.rand
func runtime_rand() uint64

Not only that, but runtime.rand is used all over the stdlib, net, os, sync, math/rand, hashing maps in the runtime, etc.

Both runtime.rand and the random source for an M (mrandinit) use globalRand that is setup in randinit, which might be using readRandom if there is no startupRand. On linux we seem to always use auxv as startupRand.

go/src/runtime/os_linux.go

Lines 245 to 250 in 71f9dbb

auxvp := (*[1 << 28]uintptr)(add(unsafe.Pointer(argv), uintptr(n)*goarch.PtrSize))
if pairs := sysauxv(auxvp[:]); pairs != 0 {
auxv = auxvp[: pairs*2 : pairs*2]
return
}

I'm not sure if the readRandom path is ever taken for linux.

@mauri870
Copy link
Member

mauri870 commented Jul 3, 2024

Looks like sysauxv has some cases where startupRand is not set, not sure how common it is:

go/src/runtime/os_linux.go

Lines 304 to 309 in 71f9dbb

case _AT_PAGESZ:
physPageSize = val
case _AT_SECURE:
secureMode = val == 1
}

In these cases it will use readRandom.

Edit: NVM, I did not saw there was a for loop here. So I think startupRand is always initialized.

@thanm thanm modified the milestones: Backlog, Go1.24 Jul 3, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/608436 mentions this issue: runtime: use arc4random_buf() for readRandom

apocelipes added a commit to apocelipes/go that referenced this issue Sep 12, 2024
Add Getrandom to internal/runtime/syscall.

Use Getrandom instead of reading /dev/urandom to simplify the code.

Since Go 1.24 needs Linux kernel >= 3.2, and getrandom(2) was
introduced by Linux kernel 3.17, Getrandom could return ENOSYS.
This doesn't matter because the Linux kernel always sets startupRand,
so readRandom is usually not reachable. We also have a time-based
fallback even if readRandom fails.

Updates golang#51087
Fixes golang#68278
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/612715 mentions this issue: runtime: use getrandom(2) for readRandom on Linux

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
Development

Successfully merging a pull request may close this issue.

8 participants