From 043f112e521dc48ec8ccffa58ceb7e5403c73fb0 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Fri, 8 Dec 2017 17:32:23 -0500 Subject: [PATCH] runtime: reset write barrier buffer on all flush paths Currently, wbBufFlush does nothing if the goroutine is dying on the assumption that the system is crashing anyway and running the write barrier may crash it even more. However, it fails to reset the buffer's "next" pointer. As a result, if there are later write barriers on the same P, the write barrier will overflow the write barrier buffer and start corrupting other fields in the P or other heap objects. Often, this corrupts fields in the next allocated P since they tend to be together in the heap. Fix this by always resetting the buffer's "next" pointer, even if we're not doing anything with the pointers in the buffer. Updates #22987 and #22988. (May fix; it's hard to say.) Change-Id: I82c11ea2d399e1658531c3e8065445a66b7282b2 Reviewed-on: https://go-review.googlesource.com/83016 Run-TryBot: Austin Clements TryBot-Result: Gobot Gobot Reviewed-by: Rick Hudson Reviewed-by: Matthew Dempsky --- src/runtime/mwbbuf.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/runtime/mwbbuf.go b/src/runtime/mwbbuf.go index 2c06996210c..4a2d1ad9880 100644 --- a/src/runtime/mwbbuf.go +++ b/src/runtime/mwbbuf.go @@ -95,6 +95,15 @@ func (b *wbBuf) reset() { } } +// discard resets b's next pointer, but not its end pointer. +// +// This must be nosplit because it's called by wbBufFlush. +// +//go:nosplit +func (b *wbBuf) discard() { + b.next = uintptr(unsafe.Pointer(&b.buf[0])) +} + // putFast adds old and new to the write barrier buffer and returns // false if a flush is necessary. Callers should use this as: // @@ -143,10 +152,14 @@ func (b *wbBuf) putFast(old, new uintptr) bool { //go:nowritebarrierrec //go:nosplit func wbBufFlush(dst *uintptr, src uintptr) { + // Note: Every possible return from this function must reset + // the buffer's next pointer to prevent buffer overflow. + if getg().m.dying > 0 { // We're going down. Not much point in write barriers // and this way we can allow write barriers in the // panic path. + getg().m.p.ptr().wbBuf.discard() return } @@ -156,8 +169,7 @@ func wbBufFlush(dst *uintptr, src uintptr) { cgoCheckWriteBarrier(dst, src) if !writeBarrier.needed { // We were only called for cgocheck. - b := &getg().m.p.ptr().wbBuf - b.next = uintptr(unsafe.Pointer(&b.buf[0])) + getg().m.p.ptr().wbBuf.discard() return } }