Skip to content

Fix use-after-free issue #10399

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
shinrich merged 3 commits into from
Sep 14, 2023
Merged

Fix use-after-free issue #10399

shinrich merged 3 commits into from
Sep 14, 2023

Conversation

@shinrich
Copy link
Member

@shinrich shinrich commented Sep 11, 2023

Details in issue description

This closes #10396

@shinrich shinrich added the Crash label Sep 11, 2023
@shinrich shinrich self-assigned this Sep 11, 2023
@shinrich
Copy link
Member Author

shinrich commented Sep 11, 2023

[approve ci autest]

@bryancall bryancall assigned cmcfarlen and unassigned cmcfarlen Sep 11, 2023
@bryancall bryancall requested a review from cmcfarlen September 11, 2023 22:06
@bryancall bryancall added this to the 10.0.0 milestone Sep 11, 2023
cmcfarlen
cmcfarlen approved these changes Sep 13, 2023
View reviewed changes
Copy link
Contributor

@cmcfarlen cmcfarlen left a comment

Choose a reason for hiding this comment

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

This PR does fix #10116 !

@cmcfarlen cmcfarlen mentioned this pull request Sep 13, 2023
Closed
@cmcfarlen
Copy link
Contributor

cmcfarlen commented Sep 13, 2023

fixes #10116

@apache apache deleted a comment from cmcfarlen Sep 14, 2023
@apache apache deleted a comment from cmcfarlen Sep 14, 2023
@shinrich shinrich merged commit 4fc1aac into apache:master Sep 14, 2023
@zwoop
Copy link
Contributor

zwoop commented Sep 14, 2023

Looks like we changed a lot around this code in master. @shinrich Can you verify that this is the correct patch for 9.2.x?

diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 6b3ef19a2..ab269fbbd 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -7346,7 +7346,11 @@ HttpSM::kill_this()
       server_txn->transaction_done();
       server_txn = nullptr;
     }
+
     if (ua_txn) {
+      if (ua_txn->get_server_session() != nullptr) {
+        ua_txn->attach_server_session(nullptr);
+      }
       ua_txn->transaction_done();
     }

zwoop pushed a commit that referenced this pull request Sep 21, 2023
@shinrich @zwoop
Fix use-after-free issue (#10399)
2f8bd20 
(cherry picked from commit 4fc1aac)

 Conflicts:
	proxy/http/HttpSM.cc
@zwoop
Copy link
Contributor

zwoop commented Sep 21, 2023

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.3 Sep 21, 2023
traeak pushed a commit to traeak/trafficserver that referenced this pull request Dec 15, 2023
@shinrich @traeak
Fix use-after-free issue (apache#10399)
3399c94 
(cherry picked from commit 4fc1aac)

 Conflicts:
	proxy/http/HttpSM.cc
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jun 3, 2024
@zwoop
Merge remote-tracking branch 'asf/master'
868b9f6 
* asf/master:
  This drops the _stat suffix from some metrics (apache#10441)
  Fix CID-1518256 (apache#10403)
  Restore original metrics names, these were typos (apache#10440)
  Use the new Metrics module for core metrics (apache#10175)
  Fix use-after-free issue (apache#10399)
  Fixed differences between cmake rc files and autotools (apache#10408)
  Fix hwloc build (apache#10406)
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Dec 10, 2024
@shinrich @cmcfarlen
Fix use-after-free issue (apache#10399)
9e8c10a 
(cherry picked from commit 4fc1aac)

 Conflicts:
	proxy/http/HttpSM.cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmcfarlen cmcfarlen cmcfarlen approved these changes

Assignees

@shinrich shinrich

Labels

Crash

Projects

None yet

Milestone

9.2.3

Development

Successfully merging this pull request may close these issues.

Use after free during transaction shutdown with attach_server_session_to_client enabled

4 participants

@shinrich @cmcfarlen @zwoop @bryancall