net/http: A race condition in the http.response.write

[nan01ab]

A race condition in the
func (w *response) write(lenData int, dataB []byte, dataS string) (n int, err error)

What version of Go are you using (go version)?

$ go version
go version go1.18.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/xxx/Library/Caches/go-build"
GOENV="/Users/xxx/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/xxx/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/xxx/go"
GOPRIVATE=""
GOPROXY="https://goproxy.cn"
GOROOT="/usr/local/Cellar/go/1.18.1/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.18.1/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.18.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/2k/9n_wwpxs7lg64xx741qrcmg40000gn/T/go-build2043652196=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

calling the Hijack() from a goroutine while the http request is being processed.

What did you expect to see?

Any Write operation on the http.ResponseWriter should return OK or ErrHijacked.
Return OK: before the Hijack() is called.
Return ErrHijacked: after the Hijack() is called.

What did you see instead?

A panic happened.

In the statement if w.conn.hijacked(), write will return ErrHijacked if conn.hijacked() is true and the conn.hijacked() is protected by conn's Mutex. But the following operations are not protected by conn's Mutex.
If the request was hijacked after if w.conn.hijacked(), The bufio.Writer in response may be modified concurrently.

func (w *response) write(lenData int, dataB []byte, dataS string) (n int, err error) {
	if w.conn.hijacked() {
              if lenData > 0 {
			caller := relevantCaller()
			w.conn.server.logf("http: response.Write on hijacked connection from %s (%s:%d)", caller.Function, path.Base(caller.File), caller.Line)
		}
		return 0, ErrHijacked
	}

	if w.canWriteContinue.isSet() {
		w.writeContinueMu.Lock()
		w.canWriteContinue.setFalse()
		w.writeContinueMu.Unlock()
	}

	if !w.wroteHeader {
		w.WriteHeader(StatusOK)
	}
	if lenData == 0 {
		return 0, nil
	}
	if !w.bodyAllowed() {
		return 0, ErrBodyNotAllowed
	}

	w.written += int64(lenData)
	if w.contentLength != -1 && w.written > w.contentLength {
		return 0, ErrContentLength
	}

	if dataB != nil {
		return w.w.Write(dataB) //   w.w may be modified concurrently.
	} else {
		return w.w.WriteString(dataS)
	}
}

[seankhliao]

please show code that can reproduce the issue

[nan01ab]

please show code that can reproduce the issue

A demo version:

package main

import (
	"io"
	"net/http"
	"sync"
)

type Handler struct {
	buf []byte
}

// just for demo
func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	w.WriteHeader(http.StatusOK)

	var wg sync.WaitGroup
	wg.Add(1) // avoid early exits
	go func() {
		defer wg.Done()
		if hijack, ok := w.(http.Hijacker); ok {
			conn, _, err := hijack.Hijack()
			if err != nil {
				return
			}
			conn.Close()
		}
	}()

	w.Write(h.buf) // should return OK or ErrHijack, not panic
	wg.Wait()
}

func main() {
	go http.ListenAndServe("127.0.0.1:8080", &Handler{
		buf: make([]byte, 4096),
	})

	var wg sync.WaitGroup
	wg.Add(8)
	for i := 0; i < 8; i++ {
		go func() {
			defer wg.Done()
			for {
				res, err := http.Get("http://127.0.0.1:8080/")
				if err != nil {
					continue
				}
				io.Copy(io.Discard, res.Body)
				res.Body.Close()
			}
		}()
	}

	wg.Wait()
}

go run hijack.go

panic:
2022/07/16 17:59:31 http: panic serving 127.0.0.1:49968: runtime error: invalid memory address or nil pointer dereference
goroutine 35063 [running]:
net/http.(*conn).serve.func1()
/usr/local/Cellar/go/1.18.1/libexec/src/net/http/server.go:1825 +0xbf
panic({0x1262060, 0x148edd0})
/usr/local/Cellar/go/1.18.1/libexec/src/runtime/panic.go:844 +0x258
bufio.(*Writer).Write(0xc0004986c0, {0xc000156000?, 0x1000?, 0x1000?})
/usr/local/Cellar/go/1.18.1/libexec/src/bufio/bufio.go:668 +0xec
net/http.(*response).write(0xc0002942a0, 0x1000, {0xc000156000, 0x1000, 0x1000}, {0x0, 0x0})
/usr/local/Cellar/go/1.18.1/libexec/src/net/http/server.go:1615 +0x21e
net/http.(*response).Write(0xc0002942a0?, {0xc000156000?, 0x0?, 0x0?})
/usr/local/Cellar/go/1.18.1/libexec/src/net/http/server.go:1573 +0x30
main.(*Handler).ServeHTTP(0xc00000e1c8, {0x131b920, 0xc0002942a0}, 0xc0001aa000?)

or:

go run hijack.go
panic: runtime error: slice bounds out of range [4105:4096]

goroutine 23757 [running]:
bufio.(*Writer).Write(0xc000229300, {0xc00031a52b?, 0xc8?, 0x2?})
/usr/local/Cellar/go/1.18.1/libexec/src/bufio/bufio.go:670 +0x1c8
net/http.writeStatusLine(0x12a4569?, 0xe?, 0x1d?, {0xc00031a52b, 0xc0005b3d10?, 0x3})
/usr/local/Cellar/go/1.18.1/libexec/src/net/http/server.go:1519 +0xbb
net/http.(*chunkWriter).writeHeader(0xc00031a4a0, {0x0, 0x0, 0x0})
/usr/local/Cellar/go/1.18.1/libexec/src/net/http/server.go:1484 +0x1098
net/http.(*chunkWriter).flush(0xc00031a4a0)
/usr/local/Cellar/go/1.18.1/libexec/src/net/http/server.go:395 +0x2b
net/http.(*response).Hijack(0xc00031a460)
/usr/local/Cellar/go/1.18.1/libexec/src/net/http/server.go:2025 +0x65
main.(*Handler).ServeHTTP.func1()