From f9085e4496eebf1bf1c4dfdcbf641d673c462c48 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sun, 26 Mar 2023 21:31:02 -0700 Subject: [PATCH] [chttp2] Fix fuzzer found flow control error (#32716) Previously we triggered a flow control update when `announced < target/2`, but if `target==1` then we fail to send a flow control update (announced is never less than 1/2==0) and break our forward progress guarantees. b/259780449 internally. --- .../chttp2/transport/flow_control.cc | 5 +- .../transport/chttp2/flow_control_fuzzer.cc | 9 +- ...mized-flow_control_fuzzer-5216952646500352 | 876 ++++++++++++++++++ 3 files changed, 887 insertions(+), 3 deletions(-) create mode 100644 test/core/transport/chttp2/flow_control_fuzzer_corpus/clusterfuzz-testcase-minimized-flow_control_fuzzer-5216952646500352 diff --git a/src/core/ext/transport/chttp2/transport/flow_control.cc b/src/core/ext/transport/chttp2/transport/flow_control.cc index 9a1602dccae..4e765f97074 100644 --- a/src/core/ext/transport/chttp2/transport/flow_control.cc +++ b/src/core/ext/transport/chttp2/transport/flow_control.cc @@ -175,7 +175,10 @@ int64_t TransportFlowControl::target_window() const { } FlowControlAction TransportFlowControl::UpdateAction(FlowControlAction action) { - if (announced_window_ < target_window() / 2) { + const int64_t target = target_window(); + // round up so that one byte targets are sent. + const int64_t send_threshold = (target + 1) / 2; + if (announced_window_ < send_threshold) { action.set_send_transport_update( FlowControlAction::Urgency::UPDATE_IMMEDIATELY); } diff --git a/test/core/transport/chttp2/flow_control_fuzzer.cc b/test/core/transport/chttp2/flow_control_fuzzer.cc index 33b8f77456a..9e4b273c086 100644 --- a/test/core/transport/chttp2/flow_control_fuzzer.cc +++ b/test/core/transport/chttp2/flow_control_fuzzer.cc @@ -404,11 +404,16 @@ void FlowControlFuzzer::AssertNoneStuck() const { fprintf(stderr, "FAILED: stream %d has stream_window=%" PRId64 ", transport_window=%" PRId64 ", delta=%" PRId64 - ", init_window_size=%" PRId64 ", min_progress_size=%" PRId64 "\n", + ", init_window_size=%" PRId64 ", min_progress_size=%" PRId64 + ", transport announced_stream_total_over_incoming_window=%" PRId64 + ", transport announced_window=%" PRId64 + " transport target_window=%" PRId64 "\n", id_stream.first, stream_window, reconciled_transport_window, reconciled_stream_deltas[id_stream.first], reconciled_initial_window, - (id_stream.second.fc.min_progress_size())); + (id_stream.second.fc.min_progress_size()), + tfc_->announced_stream_total_over_incoming_window(), + tfc_->announced_window(), tfc_->target_window()); fprintf(stderr, "initial_window breakdown: remote=%" PRId32 ", in-flight={%s}\n", remote_initial_window_size_, diff --git a/test/core/transport/chttp2/flow_control_fuzzer_corpus/clusterfuzz-testcase-minimized-flow_control_fuzzer-5216952646500352 b/test/core/transport/chttp2/flow_control_fuzzer_corpus/clusterfuzz-testcase-minimized-flow_control_fuzzer-5216952646500352 new file mode 100644 index 00000000000..2eae64a8d5c --- /dev/null +++ b/test/core/transport/chttp2/flow_control_fuzzer_corpus/clusterfuzz-testcase-minimized-flow_control_fuzzer-5216952646500352 @@ -0,0 +1,876 @@ +enable_bdp: true +actions { + stream_write { + id: 512 + } +} +actions { +} +actions { + stream_write { + id: 2 + size: 167772160 + } +} +actions { + set_min_progress_size { + size: 1 + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + step_time_ms: 99 +} +actions { +} +actions { + step_time_ms: 6712937 +} +actions { +} +actions { + step_time_ms: 0 +} +actions { +} +actions { +} +actions { + read_send_to_remote { + } +} +actions { + step_time_ms: 9018723195515904 +} +actions { +} +actions { +} +actions { +} +actions { + read_send_from_remote { + } +} +actions { + step_time_ms: 0 +} +actions { +} +actions { +} +actions { + read_send_to_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { + set_memory_quota: 537557792 +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + step_time_ms: 0 +} +actions { +} +actions { + step_time_ms: 0 +} +actions { +} +actions { + stream_write { + id: 524288 + size: 2097152 + } +} +actions { +} +actions { + periodic_update { + } +} +actions { +} +actions { +} +actions { + allocate_memory: 65 +} +actions { +} +actions { +} +actions { + periodic_update { + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + perform_send_from_remote { + } +} +actions { +} +actions { + step_time_ms: 2048 +} +actions { + step_time_ms: 0 +} +actions { +} +actions { + step_time_ms: 8386653583721889792 +} +actions { +} +actions { +} +actions { +} +actions { + step_time_ms: 281453501874176 +} +actions { +} +actions { + step_time_ms: 0 +} +actions { + step_time_ms: 0 +} +actions { + perform_send_to_remote { + } +} +actions { +} +actions { +} +actions { + allocate_memory: 4210688 +} +actions { +} +actions { +} +actions { + periodic_update { + } +} +actions { + allocate_memory: 4227858432 +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + read_send_to_remote { + } +} +actions { +} +actions { +} +actions { + read_send_from_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + step_time_ms: 16384 +} +actions { + perform_send_from_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + allocate_memory: 1028 +} +actions { + periodic_update { + } +} +actions { +} +actions { + deallocate_memory: 85 +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + periodic_update { + } +} +actions { +} +actions { + allocate_memory: 536870912 +} +actions { +} +actions { +} +actions { + step_time_ms: 16384 +} +actions { +} +actions { + periodic_update { + } +} +actions { + allocate_memory: 67174400 +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + allocate_memory: 2 +} +actions { + stream_write { + size: 4 + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + deallocate_memory: 0 +} +actions { +} +actions { +} +actions { + step_time_ms: 32 +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + step_time_ms: 16384 +} +actions { +} +actions { +} +actions { +} +actions { + allocate_memory: 45 +} +actions { + read_send_to_remote { + } +} +actions { +} +actions { + step_time_ms: 16384 +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + read_send_to_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + read_send_from_remote { + } +} +actions { +} +actions { + periodic_update { + } +} +actions { + read_send_to_remote { + } +} +actions { + stream_write { + id: 8192 + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + allocate_memory: 7 +} +actions { +} +actions { + read_send_to_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + perform_send_from_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + perform_send_to_remote { + } +} +actions { +} +actions { +} +actions { + step_time_ms: 0 +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + perform_send_from_remote { + } +} +actions { +} +actions { + perform_send_from_remote { + } +} +actions { + perform_send_to_remote { + } +} +actions { + stream_write { + id: 7 + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + perform_send_from_remote { + } +} +actions { +} +actions { +} +actions { + step_time_ms: 16445 +} +actions { +} +actions { +} +actions { + read_send_to_remote { + } +} +actions { +} +actions { +} +actions { + read_send_to_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + read_send_to_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + deallocate_memory: 4278190080 +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + read_send_to_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + perform_send_from_remote { + } +} +actions { + read_send_to_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + set_pending_size { + id: 13056 + size: 45 + } +} +actions { + allocate_memory: 13056 +} +actions { + allocate_memory: 267 +} +actions { + read_send_from_remote { + } +} +actions { + deallocate_memory: 0 +} +actions { +} +actions { +} +actions { + perform_send_to_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { + set_memory_quota: 268435456 +} +actions { +} +actions { +} +actions { +} +actions { + step_time_ms: 0 +} +actions { + read_send_to_remote { + } +} +actions { +} +actions { + step_time_ms: 16384 +} +actions { +} +actions { +} +actions { +} +actions { + allocate_memory: 0 +} +actions { +} +actions { + perform_send_from_remote { + } +} +actions { + set_memory_quota: 99 +} +actions { +} +actions { + set_memory_quota: 2 +} +actions { +} +actions { +} +actions { +} +actions { + set_memory_quota: 0 +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + read_send_from_remote { + } +} +actions { +} +actions { +} +actions { + allocate_memory: 0 +} +actions { + periodic_update { + } +} +actions { + read_send_to_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { + perform_send_to_remote { + } +} +actions { +} +actions { + perform_send_from_remote { + } +} +actions { +} +actions { +} +actions { +} +actions { + deallocate_memory: 536870912 +} +actions { + periodic_update { + } +} +actions { + periodic_update { + } +} +actions { +} +actions { +} +actions { + deallocate_memory: 4 +} +actions { +} +actions { +} +actions { + allocate_memory: 0 +} +actions { +} +actions { +} +actions { +} +actions { + allocate_memory: 2 +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { +} +actions { + perform_send_to_remote { + } +} +actions { + allocate_memory: 3211264 +} +actions { +} +actions { +} +actions { +} +actions { + deallocate_memory: 2 +} +actions { + periodic_update { + } +} +actions { + periodic_update { + } +}