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: stackFromSystem can leak memory or leave unmapped memory behind #17289

Closed
binarycrusader opened this issue Sep 29, 2016 · 4 comments

Comments

Projects
None yet
6 participants
@binarycrusader
Copy link
Contributor

commented Sep 29, 2016

When stackFromSystem = 1, the stack allocation size is rounded to the nearest page size:

 339         if debug.efence != 0 || stackFromSystem != 0 {
 340                 v := sysAlloc(round(uintptr(n), _PageSize), &memstats.stacks_sys)

...however, that rounded size is not returned to/used by the caller of stackalloc(), so when a stackfree() is later done:

 436         if debug.efence != 0 || stackFromSystem != 0 {
 437                 if debug.efence != 0 || stackFaultOnFree != 0 {
 438                         sysFault(v, n)
 439                 } else {
 440                         sysFree(v, n, &memstats.stacks_sys)
 441                 }

We'll either set less than the actual bytes allocated for faulted access, or fail to free/unmap all of the memory.

@binarycrusader

This comment has been minimized.

Copy link
Contributor Author

commented Sep 29, 2016

I suspect this isn't generally noticed on architectures that use a pagesize of 4096, since when we do a stackalloc, we'll typically do it for at least 4096, which is the nearest page size, so it usually works as expected.

As such, I suspect this is generally the most noticeable on mips64, ppc64, arm, arm64, etc.

This was discovered while working on the sparc64 port since the DefaultPhysPageSize is 8192.

@randall77

This comment has been minimized.

Copy link
Contributor

commented Sep 29, 2016

We should just increase _StackMin to at least _PageSize if stackFromSystem is set.

@quentinmit

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2016

@quentinmit quentinmit added this to the Go1.8 milestone Oct 4, 2016

@quentinmit quentinmit added the NeedsFix label Oct 4, 2016

@rsc rsc modified the milestones: Go1.9, Go1.8 Nov 11, 2016

@gopherbot

This comment has been minimized.

Copy link

commented May 18, 2017

CL https://golang.org/cl/43636 mentions this issue.

@aclements aclements removed the NeedsFix label May 18, 2017

@gopherbot gopherbot closed this in 8a1c5b2 May 23, 2017

@golang golang locked and limited conversation to collaborators May 23, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.