]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: avoid overflow in (*mheap).grow
authorMichael Anthony Knyszek <mknyszek@google.com>
Wed, 6 May 2020 19:18:07 +0000 (19:18 +0000)
committerMichael Knyszek <mknyszek@google.com>
Thu, 7 May 2020 21:39:50 +0000 (21:39 +0000)
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 <austin@google.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/runtime/mheap.go

index b7c5add40c5fbc4828b5f168bbb69bc519bbed76..558ff1f6899e92fa638c4819e210e6b8189408ce 100644 (file)
@@ -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)
        }