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

Jump to bottom

runtime: pageAlloc.searchAddr may point to unmapped memory in discontiguous heaps, violating its invariant [1.14 backport] #40192

Closed
gopherbot opened this issue Jul 13, 2020 · 5 comments
Closed

runtime: pageAlloc.searchAddr may point to unmapped memory in discontiguous heaps, violating its invariant [1.14 backport] #40192

gopherbot opened this issue Jul 13, 2020 · 5 comments
Labels
CherryPickApproved FrozenDueToAge
Milestone
Go1.14.9

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 13, 2020

@mknyszek requested issue #40191 to be considered for backport to the next 1.14 minor release.

@gopherbot Please open a backport issue to Go 1.14.
@gopherbot gopherbot added the CherryPickCandidate label Jul 13, 2020
@gopherbot gopherbot mentioned this issue Jul 13, 2020
Closed
@gopherbot gopherbot added this to the Go1.14.5 milestone Jul 13, 2020
@andybons andybons removed this from the Go1.14.5 milestone Jul 14, 2020
@andybons andybons added this to the Go1.14.6 milestone Jul 14, 2020
@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented Jul 14, 2020 •
edited

@mknyszek Can you please include a short rationale about why the backport might be needed? (Per MinorReleases.) Thanks.

@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Jul 14, 2020

Yes, sorry. I mentioned it in #40191 but forgot to copy it here.

This issue should not be considered a blocking issue for Go 1.15 because the bug was technically introduced in Go 1.14, but it should be fixed and probably backported too, since it can cause failures with no workaround.

Specifically, this bug causes a segfault that can happen to anyone for reasons outside of their control, with no workaround at the user level. Although it's generally very rare because OSes generally try to keep the mapped regions contiguous, if future versions of e.g. Linux decided not to try so hard to keep mmap'd regions contiguous (and it ignored all our hints), then someone might see a consistent segfault. Currently this only happens on Fuchsia (a port we don't support ourselves) but the problem theoretically applies to all platforms just at varying degrees of rarity.

@andybons andybons removed this from the Go1.14.6 milestone Jul 16, 2020
@andybons andybons added this to the Go1.14.7 milestone Jul 16, 2020
@toothrot
Copy link
Contributor

@toothrot toothrot commented Jul 23, 2020

Approving for backport. This is a serious issue with no workaround.

@toothrot toothrot added the CherryPickApproved label Jul 23, 2020
@gopherbot gopherbot removed the CherryPickCandidate label Jul 23, 2020
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Jul 31, 2020

Change https://golang.org/cl/246197 mentions this issue: [release-branch.go.1.14] runtime: validate candidate searchAddr in pageAlloc.find

@toothrot toothrot removed this from the Go1.14.7 milestone Aug 6, 2020
@toothrot toothrot added this to the Go1.14.8 milestone Aug 6, 2020
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Aug 22, 2020

Closed by merging 060e0c8 to release-branch.go1.14.

gopherbot pushed a commit that referenced this issue Aug 22, 2020
@mknyszek @dmitshur
[release-branch.go.1.14] runtime: validate candidate searchAddr in pa…
060e0c8 
…geAlloc.find

Currently pageAlloc.find attempts to find a better estimate for the
first free page in the heap, even if the space its looking for isn't
necessarily going to be the first free page in the heap (e.g. if npages
>= 2). However, in doing so it has the potential to return a searchAddr
candidate that doesn't actually correspond to mapped memory, but this
candidate might still be adopted. As a result, pageAlloc.alloc's fast
path may look at unmapped summary memory and segfault. This case is rare
on most operating systems since the heap is kept fairly contiguous, so
the chance that the candidate searchAddr discovered is unmapped is
fairly low. Even so, this is totally possible and outside the user's
control when it happens (in fact, it's likely to happen consistently for
a given user on a given system).

Fix this problem by ensuring that our candidate always points to mapped
memory. We do this by looking at mheap's arenas structure first. If it
turns out our candidate doesn't correspond to mapped memory, then we
look at inUse to round up the searchAddr to the next mapped address.

While we're here, clean up some documentation related to searchAddr.

For #40191.
Fixes #40192.

Change-Id: I759efec78987e4a8fde466ae45aabbaa3d9d4214
Reviewed-on: https://go-review.googlesource.com/c/go/+/242680
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit b56791c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/246197
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
@dmitshur dmitshur removed this from the Go1.14.8 milestone Sep 1, 2020
@dmitshur dmitshur added this to the Go1.14.9 milestone Sep 1, 2020
@golang golang locked and limited conversation to collaborators Sep 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved FrozenDueToAge
Projects
None yet
Milestone
Go1.14.9
Development

No branches or pull requests
5 participants
@toothrot @andybons @mknyszek @dmitshur @gopherbot