From 14ae846f54c105f4d48f1afa5aa5446e4b9e7cdc Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Wed, 6 May 2020 19:18:07 +0000 Subject: [PATCH] runtime: avoid overflow in (*mheap).grow Currently when checking if we can grow the heap into the current arena, we do an addition which may overflow. This is particularly likely on 32-bit systems. Avoid this situation by explicitly checking for overflow, and adding in some comments about when overflow is possible, when it isn't, and why. For #35954. Change-Id: I2d4ecbb1ccbd43da55979cc721f0cd8d1757add2 Reviewed-on: https://go-review.googlesource.com/c/go/+/231337 Reviewed-by: Austin Clements Reviewed-by: David Chase Run-TryBot: Michael Knyszek TryBot-Result: Gobot Gobot --- src/runtime/mheap.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index b7c5add40c5..558ff1f6899 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -1327,8 +1327,11 @@ func (h *mheap) grow(npage uintptr) bool { ask := alignUp(npage, pallocChunkPages) * pageSize totalGrowth := uintptr(0) - nBase := alignUp(h.curArena.base+ask, physPageSize) - if nBase > h.curArena.end { + // This may overflow because ask could be very large + // and is otherwise unrelated to h.curArena.base. + end := h.curArena.base + ask + nBase := alignUp(end, physPageSize) + if nBase > h.curArena.end || /* overflow */ end < h.curArena.base { // Not enough room in the current arena. Allocate more // arena space. This may not be contiguous with the // current arena, so we have to request the full ask. @@ -1364,7 +1367,10 @@ func (h *mheap) grow(npage uintptr) bool { mSysStatInc(&memstats.heap_released, asize) mSysStatInc(&memstats.heap_idle, asize) - // Recalculate nBase + // Recalculate nBase. + // We know this won't overflow, because sysAlloc returned + // a valid region starting at h.curArena.base which is at + // least ask bytes in size. nBase = alignUp(h.curArena.base+ask, physPageSize) }