Skip to content

Fix another hidden use-after-free in CallTraceStorage #301

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.

Sign up for GitHub

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

Jump to bottom
Merged
jbachorik merged 1 commit into from
Nov 28, 2025
Merged

Fix another hidden use-after-free in CallTraceStorage #301

jbachorik merged 1 commit into from
Nov 28, 2025

Conversation

@jbachorik
Copy link
Collaborator

@jbachorik jbachorik commented Nov 26, 2025 •
edited
Loading

What does this PR do?:
Here we address the (almost certain) last use-after-free bug in CallTraceStorage implementation

Additional Notes:
The gist of the fix is to make the LinearAllocator cleanup 2-phased - first just reset the allocator to start handing out new chunks and actually deallocate the previously used chunks only when it is safe, after the all the collected traces had been processed.

I have generated the reproducer test, hence the more verbose comments, but I think they might be useful for history keeping. The test is using ASAN poisoning and will reliably crash when run under ASAN.

How to test the change?:
Added a new gtest which is supposed to be run under ASAN.

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.
  • JIRA: PROF-13156

Unsure? Have a question? Request a review!

@jbachorik jbachorik force-pushed the jb/cts_mem_corruption branch from 7c08229 to a3a068e Compare November 26, 2025 22:07
@jbachorik jbachorik requested a review from zhengyu123 November 26, 2025 22:21
@jbachorik jbachorik marked this pull request as ready for review November 26, 2025 22:21
@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [x86_64 cpu]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes cpu cpu
wall off off

Summary

Found 0 performance improvements and 4 performance regressions! Performance is the same for 12 metrics, 22 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:finagle-http worse
[+716.878ms; +1063.122ms] or [+2.710%; +4.019%]		 unstable
[-267.741MB; +374.601MB] or [-19.422%; +27.173%]
scenario:renaissance:future-genetic worse
[+726.502ms; +1273.498ms] or [+4.585%; +8.038%]		 unstable
[-312.707MB; +413.666MB] or [-32.002%; +42.334%]
scenario:renaissance:scala-kmeans worse
[+0.642s; +1.454s] or [+2.791%; +6.319%]		 unstable
[-229.923MB; +339.593MB] or [-23.175%; +34.228%]
scenario:renaissance:gauss-mix worse
[+678.192ms; +829.808ms] or [+3.727%; +4.560%]		 unstable
[-400.615MB; +502.433MB] or [-33.596%; +42.134%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [x86_64 alloc]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes alloc alloc
wall off off

Summary

Found 0 performance improvements and 2 performance regressions! Performance is the same for 13 metrics, 23 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:fj-kmeans worse
[+636.494ms; +735.506ms] or [+2.742%; +3.169%]		 unstable
[-245.575MB; +358.199MB] or [-23.352%; +34.062%]
scenario:renaissance:scala-kmeans worse
[+747.150ms; +1184.850ms] or [+3.264%; +5.176%]		 unstable
[-229.869MB; +340.129MB] or [-23.158%; +34.267%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [aarch64 wall]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes wall wall
wall on on

Summary

Found 0 performance improvements and 4 performance regressions! Performance is the same for 14 metrics, 20 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:finagle-chirper worse
[+1.182s; +2.814s] or [+3.651%; +8.692%]		 unstable
[-229.161MB; +316.807MB] or [-16.503%; +22.815%]
scenario:renaissance:future-genetic worse
[+428.225ms; +843.775ms] or [+2.860%; +5.636%]		 unstable
[-258.295MB; +526.180MB] or [-29.595%; +60.288%]
scenario:renaissance:chi-square worse
[+333.098ms; +1234.902ms] or [+2.118%; +7.851%]		 unstable
[-369.550MB; +461.391MB] or [-33.394%; +41.693%]
scenario:renaissance:dec-tree worse
[+453.018ms; +622.982ms] or [+1.502%; +2.065%]		 unstable
[-188.364MB; +424.858MB] or [-13.956%; +31.478%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [aarch64 cpu]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes cpu cpu
wall off off

Summary

Found 0 performance improvements and 3 performance regressions! Performance is the same for 13 metrics, 22 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:future-genetic worse
[+488.066ms; +695.934ms] or [+3.249%; +4.632%]		 unstable
[-262.079MB; +525.854MB] or [-29.865%; +59.924%]
scenario:renaissance:chi-square worse
[+432.351ms; +951.649ms] or [+2.698%; +5.938%]		 unstable
[-362.096MB; +472.379MB] or [-32.748%; +42.721%]
scenario:renaissance:fj-kmeans worse
[+384.217ms; +763.783ms] or [+1.820%; +3.618%]		 unstable
[-231.732MB; +363.017MB] or [-22.567%; +35.352%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [aarch64 cpu,wall,alloc,memleak]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes cpu,wall,alloc,memleak cpu,wall,alloc,memleak
wall on on

Summary

Found 0 performance improvements and 2 performance regressions! Performance is the same for 16 metrics, 20 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:future-genetic worse
[+460.085ms; +647.915ms] or [+3.054%; +4.301%]		 unstable
[-248.195MB; +565.273MB] or [-29.092%; +66.259%]
scenario:renaissance:scala-kmeans worse
[+507.321ms; +572.679ms] or [+2.105%; +2.376%]		 unstable
[-226.295MB; +340.016MB] or [-23.061%; +34.649%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [x86_64 wall]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes wall wall
wall on on

Summary

Found 0 performance improvements and 5 performance regressions! Performance is the same for 8 metrics, 25 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:finagle-http worse
[+753.098ms; +1058.902ms] or [+2.866%; +4.029%]		 unstable
[-257.387MB; +373.234MB] or [-19.042%; +27.613%]
scenario:renaissance:future-genetic worse
[+379.767ms; +1212.233ms] or [+2.398%; +7.654%]		 unstable
[-306.499MB; +417.514MB] or [-31.558%; +42.988%]
scenario:renaissance:fj-kmeans worse
[+524.340ms; +679.660ms] or [+2.256%; +2.924%]		 unstable
[-241.783MB; +355.793MB] or [-23.239%; +34.197%]
scenario:renaissance:scala-kmeans worse
[+479.409ms; +948.591ms] or [+2.073%; +4.102%]		 unstable
[-227.865MB; +337.667MB] or [-23.134%; +34.281%]
scenario:renaissance:gauss-mix worse
[+849.543ms; +922.457ms] or [+4.713%; +5.118%]		 unstable
[-394.178MB; +506.115MB] or [-33.236%; +42.675%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [aarch64 cpu,wall]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes cpu,wall cpu,wall
wall on on

Summary

Found 0 performance improvements and 2 performance regressions! Performance is the same for 16 metrics, 20 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:future-genetic worse
[+499.755ms; +608.245ms] or [+3.318%; +4.039%]		 unstable
[-247.092MB; +566.994MB] or [-28.957%; +66.447%]
scenario:renaissance:par-mnemonics worse
[+0.382s; +2.354s] or [+1.699%; +10.460%]		 unstable
[-228.545MB; +356.127MB] or [-22.097%; +34.433%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [aarch64 memleak]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes memleak memleak
wall off off

Summary

Found 0 performance improvements and 4 performance regressions! Performance is the same for 13 metrics, 21 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:finagle-chirper worse
[+1.006s; +1.958s] or [+3.056%; +5.946%]		 unstable
[-222.464MB; +335.204MB] or [-15.789%; +23.791%]
scenario:renaissance:future-genetic worse
[+520.737ms; +675.263ms] or [+3.467%; +4.496%]		 unstable
[-250.081MB; +563.149MB] or [-29.299%; +65.978%]
scenario:renaissance:chi-square worse
[+287.366ms; +1200.634ms] or [+1.825%; +7.626%]		 unstable
[-367.450MB; +470.588MB] or [-33.053%; +42.331%]
scenario:renaissance:naive-bayes worse
[+0.682s; +1.410s] or [+4.607%; +9.528%]		 unstable
[-252.864MB; +678.670MB] or [-26.456%; +71.007%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [aarch64 alloc]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes alloc alloc
wall off off

Summary

Found 0 performance improvements and 4 performance regressions! Performance is the same for 13 metrics, 21 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:future-genetic worse
[+467.547ms; +680.453ms] or [+3.110%; +4.526%]		 unstable
[-250.158MB; +564.008MB] or [-29.278%; +66.010%]
scenario:renaissance:chi-square worse
[+474.409ms; +1453.591ms] or [+3.059%; +9.373%]		 unstable
[-333.426MB; +530.453MB] or [-31.608%; +50.285%]
scenario:renaissance:fj-kmeans worse
[+567.387ms; +824.613ms] or [+2.704%; +3.930%]		 unstable
[-241.557MB; +357.612MB] or [-23.257%; +34.431%]
scenario:renaissance:als worse
[+0.611s; +2.009s] or [+1.629%; +5.355%]		 unstable
[-193.292MB; +312.649MB] or [-13.416%; +21.701%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [x86_64 memleak]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes memleak memleak
wall off off

Summary

Found 0 performance improvements and 3 performance regressions! Performance is the same for 12 metrics, 23 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:fj-kmeans worse
[+381.094ms; +542.906ms] or [+1.627%; +2.317%]		 unstable
[-248.293MB; +357.337MB] or [-23.510%; +33.835%]
scenario:renaissance:scala-kmeans worse
[+531.203ms; +1228.797ms] or [+2.312%; +5.347%]		 unstable
[-226.470MB; +343.548MB] or [-22.854%; +34.669%]
scenario:renaissance:gauss-mix worse
[+736.501ms; +987.499ms] or [+4.071%; +5.458%]		 unstable
[-390.067MB; +514.714MB] or [-32.804%; +43.287%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [x86_64 cpu,wall]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc off off
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak off off
modes cpu,wall cpu,wall
wall on on

Summary

Found 0 performance improvements and 4 performance regressions! Performance is the same for 11 metrics, 23 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:finagle-http worse
[+523.397ms; +640.603ms] or [+1.956%; +2.394%]		 unstable
[-257.649MB; +380.966MB] or [-18.844%; +27.863%]
scenario:renaissance:future-genetic worse
[+588.661ms; +1199.339ms] or [+3.690%; +7.518%]		 unstable
[-308.947MB; +417.826MB] or [-31.670%; +42.832%]
scenario:renaissance:akka-uct worse
[+0.470s; +2.070s] or [+1.727%; +7.599%]		 unstable
[-212.484MB; +364.792MB] or [-17.302%; +29.704%]
scenario:renaissance:gauss-mix worse
[+694.016ms; +949.984ms] or [+3.826%; +5.238%]		 unstable
[-394.285MB; +509.281MB] or [-33.146%; +42.813%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [x86_64 memleak,alloc]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes memleak,alloc memleak,alloc
wall off off

Summary

Found 0 performance improvements and 2 performance regressions! Performance is the same for 12 metrics, 24 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:finagle-http worse
[+563.019ms; +824.981ms] or [+2.113%; +3.096%]		 unstable
[-263.403MB; +378.589MB] or [-19.154%; +27.530%]
scenario:renaissance:gauss-mix worse
[+656.439ms; +811.561ms] or [+3.604%; +4.455%]		 unstable
[-394.402MB; +510.280MB] or [-33.125%; +42.857%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [x86_64 cpu,wall,alloc,memleak]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu on on
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes cpu,wall,alloc,memleak cpu,wall,alloc,memleak
wall on on

Summary

Found 0 performance improvements and 4 performance regressions! Performance is the same for 10 metrics, 24 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:finagle-http worse
[+489.249ms; +922.751ms] or [+1.837%; +3.464%]		 unstable
[-269.582MB; +371.644MB] or [-19.567%; +26.975%]
scenario:renaissance:future-genetic worse
[+300.320ms; +1315.680ms] or [+1.896%; +8.305%]		 unstable
[-310.482MB; +416.448MB] or [-31.796%; +42.647%]
scenario:renaissance:dec-tree worse
[+854.387ms; +1025.613ms] or [+2.750%; +3.301%]		 unstable
[-233.332MB; +358.285MB] or [-16.123%; +24.758%]
scenario:renaissance:gauss-mix worse
[+654.506ms; +877.494ms] or [+3.598%; +4.824%]		 unstable
[-398.563MB; +505.570MB] or [-33.408%; +42.377%]

@pr-commenter
Copy link

pr-commenter bot commented Nov 26, 2025 •
edited
Loading

Benchmarks [aarch64 memleak,alloc]

Parameters

Baseline Candidate
config baseline candidate
ddprof 1.34.3 1.35.0-jb_cts_mem_corruption-SNAPSHOT
See matching parameters
Baseline Candidate
alloc on on
cpu off off
iterations 5 5
java "11.0.28" "11.0.28"
memleak on on
modes memleak,alloc memleak,alloc
wall off off

Summary

Found 0 performance improvements and 3 performance regressions! Performance is the same for 14 metrics, 21 unstable metrics.

scenario Δ mean execution_time Δ mean rss
scenario:renaissance:future-genetic worse
[+489.962ms; +710.038ms] or [+3.261%; +4.726%]		 unstable
[-254.225MB; +560.325MB] or [-29.689%; +65.435%]
scenario:renaissance:chi-square worse
[+687.665ms; +848.335ms] or [+4.312%; +5.319%]		 unstable
[-372.428MB; +466.227MB] or [-33.396%; +41.806%]
scenario:renaissance:philosophers worse
[+337.035ms; +1246.965ms] or [+3.350%; +12.395%]		 unstable
[-342.730MB; +779.408MB] or [-44.669%; +101.582%]

@jbachorik jbachorik requested a review from rkennke November 28, 2025 08:45
rkennke
rkennke approved these changes Nov 28, 2025
View reviewed changes
Copy link
Contributor

@rkennke rkennke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@jbachorik jbachorik merged commit e800295 into main Nov 28, 2025
95 checks passed
@jbachorik jbachorik deleted the jb/cts_mem_corruption branch November 28, 2025 10:59
@github-actions github-actions bot added this to the 1.35.0 milestone Nov 28, 2025
jbachorik added a commit that referenced this pull request Dec 5, 2025
@jbachorik
Fix another hidden use-after-free in CallTraceStorage (#301)
06130aa 
(cherry picked from commit e800295)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rkennke rkennke rkennke approved these changes

@zhengyu123 zhengyu123 Awaiting requested review from zhengyu123

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.35.0

Development

Successfully merging this pull request may close these issues.

3 participants

@jbachorik @rkennke