Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup before freeing FakeStream to prevent use-after-free #38509

Merged
merged 1 commit into from
Feb 21, 2025

Cleanup before freeing FakeStream to prevent use-after-free

0c2ce96
Select commit
Loading
Failed to load commit list.
Merged

Cleanup before freeing FakeStream to prevent use-after-free #38509

Cleanup before freeing FakeStream to prevent use-after-free
0c2ce96
Select commit
Loading
Failed to load commit list.
CI (Envoy) / Mobile/TSAN skipped Feb 20, 2025 in 0s

Check was skipped

This check was not triggered in this CI run

Details

Request (pr/38509/main@0c2ce96)

krinkinmu @krinkinmu 0c2ce96 #38509 merge main@f1e276f

Cleanup before freeing FakeStream to prevent use-after-free

Commit Message:

In SendGoAwayNotTriggerredByDecodingFilter it could happen that FakeStream will get destroyed (when the test ends) before FakeUpstream dispatcher thread (FakeUpstream::threadRoutine()) exits or before all processing on the connection is complete.

That leads to a situation when FakeUpstream dispatcher thread may try to call into the FakeStream::onResetStream when connection established by the test gets terminated after FakeStream was already destroyed.

This triggers MSAN issue on clang-18 pretty consistently, though I don't think it's deterministic and could potentially happen on clang-14 currently used by Envoy CIs.

This change makes sure that we do cleanup before exiting the test, thus preventing the use-after-free issue.

Additional Description:

Related to #37911 and fixes one of the issues that block clang-18 adoption in Envoy CI.

Risk Level: low
Testing: do_ci.sh msan using Envoy CI image with clang-18
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

Environment

Request variables

Key Value
ref 37f93dc
sha 0c2ce96
pr 38509
base-sha f1e276f
actor krinkinmu @krinkinmu
message Cleanup before freeing FakeStream to prevent use-after-free...
started 1740078835.559974
target-branch main
trusted false
Build image

Container image/s (as used in this CI run)

Key Value
default envoyproxy/envoy-build-ubuntu:506aa20d3a216fa2f5677c1f13d2b1656b000b86
mobile envoyproxy/envoy-build-ubuntu:mobile-506aa20d3a216fa2f5677c1f13d2b1656b000b86
Version

Envoy version (as used in this CI run)

Key Value
major 1
minor 34
patch 0
dev true