You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by sh...@apache.org on 2015/06/04 20:10:28 UTC

trafficserver git commit: TS-3378: SpdyRequest used after free. This closes #211

Repository: trafficserver
Updated Branches:
  refs/heads/master e8b38aea0 -> 3e6b4b920


TS-3378: SpdyRequest used after free.
This closes #211


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/3e6b4b92
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/3e6b4b92
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/3e6b4b92

Branch: refs/heads/master
Commit: 3e6b4b9204520c75aac19c87063ce6d403a256cb
Parents: e8b38ae
Author: shinrich <sh...@yahoo-inc.com>
Authored: Wed Jun 3 15:09:58 2015 -0500
Committer: shinrich <sh...@yahoo-inc.com>
Committed: Thu Jun 4 13:09:33 2015 -0500

----------------------------------------------------------------------
 CHANGES                         |  2 ++
 proxy/spdy/SpdyCallbacks.cc     |  2 +-
 proxy/spdy/SpdyClientSession.cc | 24 +++++++++++++++---------
 3 files changed, 18 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3e6b4b92/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 965a907..6356444 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 6.0.0
 
+  *) [TS-3378] SpdyRequest used after free()
+
   *) [TS-3664] WebSockets attempts a cache lookup and blocks on multiple requests
 
   *) [TS-3661] keep cache_sm active during 3xx redirect follow to allow

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3e6b4b92/proxy/spdy/SpdyCallbacks.cc
----------------------------------------------------------------------
diff --git a/proxy/spdy/SpdyCallbacks.cc b/proxy/spdy/SpdyCallbacks.cc
index a625fd7..e7b529a 100644
--- a/proxy/spdy/SpdyCallbacks.cc
+++ b/proxy/spdy/SpdyCallbacks.cc
@@ -188,7 +188,7 @@ spdy_fetcher_launch(SpdyRequest *req)
   fetch_flags |= TS_FETCH_FLAGS_NOT_INTERNAL_REQUEST;
 
   req->fetch_sm = TSFetchCreate((TSCont)sm, req->method.c_str(), url.c_str(), req->version.c_str(), client_addr, fetch_flags);
-  TSFetchUserDataSet(req->fetch_sm, req);
+  TSFetchUserDataSet(req->fetch_sm, (void *)req);
 
   //
   // Set header list

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/3e6b4b92/proxy/spdy/SpdyClientSession.cc
----------------------------------------------------------------------
diff --git a/proxy/spdy/SpdyClientSession.cc b/proxy/spdy/SpdyClientSession.cc
index b039892..963ab03 100644
--- a/proxy/spdy/SpdyClientSession.cc
+++ b/proxy/spdy/SpdyClientSession.cc
@@ -43,8 +43,8 @@ static char const *const npnmap[] = {TS_NPN_PROTOCOL_SPDY_2, TS_NPN_PROTOCOL_SPD
 static int spdy_process_read(TSEvent event, SpdyClientSession *sm);
 static int spdy_process_write(TSEvent event, SpdyClientSession *sm);
 static int spdy_process_fetch(TSEvent event, SpdyClientSession *sm, void *edata);
-static int spdy_process_fetch_header(TSEvent event, SpdyClientSession *sm, TSFetchSM fetch_sm);
-static int spdy_process_fetch_body(TSEvent event, SpdyClientSession *sm, TSFetchSM fetch_sm);
+static int spdy_process_fetch_header(TSEvent event, SpdyClientSession *sm, TSFetchSM fetch_sm, SpdyRequest *req);
+static int spdy_process_fetch_body(TSEvent event, SpdyClientSession *sm, TSFetchSM fetch_sm, SpdyRequest *req);
 static uint64_t g_sm_id = 1;
 
 void
@@ -69,6 +69,10 @@ SpdyRequest::clear()
   SPDY_DECREMENT_THREAD_DYN_STAT(SPDY_STAT_CURRENT_CLIENT_STREAM_COUNT, spdy_sm->mutex->thread_holding);
 
   if (fetch_sm) {
+    // Clear the UserData just in case fetch_sm's
+    // death is delayed.  Don't want freed requests 
+    // showing up in callbacks
+    TSFetchUserDataSet(fetch_sm, NULL);
     TSFetchDestroy(fetch_sm);
     fetch_sm = NULL;
   }
@@ -323,22 +327,26 @@ spdy_process_fetch(TSEvent event, SpdyClientSession *sm, void *edata)
   int ret = -1;
   TSFetchSM fetch_sm = (TSFetchSM)edata;
   SpdyRequest *req = (SpdyRequest *)TSFetchUserDataGet(fetch_sm);
+  if (!req) {
+    Warning("spdy_process_fetch: stream already gone");
+    return ret;
+  }
 
   switch ((int)event) {
   case TS_FETCH_EVENT_EXT_HEAD_DONE:
     Debug("spdy", "----[FETCH HEADER DONE]");
-    ret = spdy_process_fetch_header(event, sm, fetch_sm);
+    ret = spdy_process_fetch_header(event, sm, fetch_sm, req);
     break;
 
   case TS_FETCH_EVENT_EXT_BODY_READY:
     Debug("spdy", "----[FETCH BODY READY]");
-    ret = spdy_process_fetch_body(event, sm, fetch_sm);
+    ret = spdy_process_fetch_body(event, sm, fetch_sm, req);
     break;
 
   case TS_FETCH_EVENT_EXT_BODY_DONE:
     Debug("spdy", "----[FETCH BODY DONE]");
     req->fetch_body_completed = true;
-    ret = spdy_process_fetch_body(event, sm, fetch_sm);
+    ret = spdy_process_fetch_body(event, sm, fetch_sm, req);
     break;
 
   default:
@@ -364,10 +372,9 @@ spdy_process_fetch(TSEvent event, SpdyClientSession *sm, void *edata)
 }
 
 static int
-spdy_process_fetch_header(TSEvent /*event*/, SpdyClientSession *sm, TSFetchSM fetch_sm)
+spdy_process_fetch_header(TSEvent /*event*/, SpdyClientSession *sm, TSFetchSM fetch_sm, SpdyRequest *req)
 {
   int ret = -1;
-  SpdyRequest *req = (SpdyRequest *)TSFetchUserDataGet(fetch_sm);
 
   SpdyNV spdy_nv(fetch_sm);
 
@@ -445,11 +452,10 @@ spdy_read_fetch_body_callback(spdylay_session * /*session*/, int32_t stream_id,
 }
 
 static int
-spdy_process_fetch_body(TSEvent event, SpdyClientSession *sm, TSFetchSM fetch_sm)
+spdy_process_fetch_body(TSEvent event, SpdyClientSession *sm, TSFetchSM fetch_sm, SpdyRequest *req)
 {
   int ret = 0;
   spdylay_data_provider data_prd;
-  SpdyRequest *req = (SpdyRequest *)TSFetchUserDataGet(fetch_sm);
   req->event = event;
 
   data_prd.source.ptr = (void *)req;