Skip to content

Commit 64b3af9

Browse files
committed
http2: prevent transport deadlock due to WINDOW_UPDATE that exceeds limit
Transport currently deadlocks when receiving a WINDOW_UPDATE for a non-zero stream that increments its window beyond the 2^31-1 bytes limit. This is because endStreamError is called to end the non-zero stream, which tries to lock an already-locked mutex. Therefore, create and use endStreamErrorLocked instead, which assumes the mutex is already locked. Fixes golang/go#77331 Change-Id: Iea212f49a1f305d1bddefb8831dbaca00840870c Reviewed-on: https://go-review.googlesource.com/c/net/+/739700 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Nicholas Husin <husin@google.com> Reviewed-by: Damien Neil <dneil@google.com>
1 parent 1973e8d commit 64b3af9

File tree

2 files changed

+29
-1
lines changed

2 files changed

+29
-1
lines changed

http2/transport.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2779,6 +2779,11 @@ func (rl *clientConnReadLoop) endStreamError(cs *clientStream, err error) {
27792779
cs.abortStream(err)
27802780
}
27812781

2782+
func (rl *clientConnReadLoop) endStreamErrorLocked(cs *clientStream, err error) {
2783+
cs.readAborted = true
2784+
cs.abortStreamLocked(err)
2785+
}
2786+
27822787
// Constants passed to streamByID for documentation purposes.
27832788
const (
27842789
headerOrDataFrame = true
@@ -2946,7 +2951,7 @@ func (rl *clientConnReadLoop) processWindowUpdate(f *WindowUpdateFrame) error {
29462951
if !fl.add(int32(f.Increment)) {
29472952
// For stream, the sender sends RST_STREAM with an error code of FLOW_CONTROL_ERROR
29482953
if cs != nil {
2949-
rl.endStreamError(cs, StreamError{
2954+
rl.endStreamErrorLocked(cs, StreamError{
29502955
StreamID: f.StreamID,
29512956
Code: ErrCodeFlowControl,
29522957
})

http2/transport_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1881,6 +1881,29 @@ func testTransportResponseHeaderTimeout(t testing.TB, body bool) {
18811881
}
18821882
}
18831883

1884+
// https://go.dev/issue/77331
1885+
func TestTransportWindowUpdateBeyondLimit(t *testing.T) {
1886+
synctestTest(t, testTransportWindowUpdateBeyondLimit)
1887+
}
1888+
func testTransportWindowUpdateBeyondLimit(t testing.TB) {
1889+
const windowIncrease uint32 = (1 << 31) - 1 // Will cause window to exceed limit of 2^31-1.
1890+
tc := newTestClientConn(t)
1891+
tc.greet()
1892+
1893+
req, _ := http.NewRequest("GET", "https://dummy.tld/", nil)
1894+
rt := tc.roundTrip(req)
1895+
tc.wantHeaders(wantHeader{
1896+
streamID: rt.streamID(),
1897+
endStream: true,
1898+
})
1899+
1900+
tc.writeWindowUpdate(rt.streamID(), windowIncrease)
1901+
tc.wantRSTStream(rt.streamID(), ErrCodeFlowControl)
1902+
1903+
tc.writeWindowUpdate(0, windowIncrease)
1904+
tc.wantClosed()
1905+
}
1906+
18841907
func TestTransportDisableCompression(t *testing.T) {
18851908
const body = "sup"
18861909
ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) {

0 commit comments

Comments
 (0)