Fix use-after-free in CreateDefaultDisplayReporter (#2240)#2265
Open
devtejasx wants to merge 1 commit into
Open
Fix use-after-free in CreateDefaultDisplayReporter (#2240)#2265devtejasx wants to merge 1 commit into
devtejasx wants to merge 1 commit into
Conversation
CreateDefaultDisplayReporter() transfers ownership of the returned reporter to the caller: it calls .release() and RunSpecifiedBenchmarks() wraps the result in a unique_ptr that deletes it on return. The implementation, however, cached that owning pointer in a function-local static. After the first RunSpecifiedBenchmarks() call deleted the reporter, the second call returned the same, now-dangling pointer, causing a heap-use-after-free in BenchmarkReporter::GetOutputStream(). Create a fresh reporter on every call so each caller owns and deletes an independent object. Add a regression test asserting successive calls return distinct, individually-owned reporters. AI-assisted: patch drafted with AI assistance (Claude); reviewed, tested, and understood by me. I take full responsibility for it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2240.
CreateDefaultDisplayReporter()transfers ownership of the returned reporter tothe caller: it calls
.release(), andRunSpecifiedBenchmarks()wraps theresult in a
unique_ptrthat deletes it on return. The implementation, however,cached that owning pointer in a function-local
static. After the firstRunSpecifiedBenchmarks()call deleted the reporter, a subsequent call returnedthe same, now-dangling pointer, causing a heap-use-after-free in
BenchmarkReporter::GetOutputStream().This removes the
staticso each call returns a fresh, caller-owned reporter,matching the function's name and its
.release()contract.Note: I'm aware the reproducer relied on calling
RunSpecifiedBenchmarks()morethan once, which isn't the primary intended usage. The change here is really an
ownership fix -- a
Create*function that hands ownership to the caller shouldnot also cache that pointer -- and it makes repeated calls safe as a side
effect. Happy to drop the regression test if you'd rather not codify multi-call
behavior.
Verification
two successive calls (the one the caller has already deleted); the new code
returns two distinct, independently-owned reporters.
create_default_display_reporter_gtestasserting successive calls returndistinct reporters; full build passes.
Per AGENTS.md: AI-assisted — the patch was drafted with AI assistance
(Claude) and then reviewed, tested, and understood by me. I take full
responsibility for it.