This is an automated email from the ASF dual-hosted git repository.

bcall pushed a commit to branch 7.0.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git

The following commit(s) were added to refs/heads/7.0.x by this push:
       new  ab243cb   TS-4938: Avoid crashes due to NULL vc dereferences.
ab243cb is described below

commit ab243cb2efa316871d275fcb0ab889d996554cfa
Author: Susan Hinrichs <shinr...@ieee.org>
AuthorDate: Thu Oct 6 16:20:08 2016 +0000

    TS-4938: Avoid crashes due to NULL vc dereferences.
    
    One more null check.
    
    Remove server_session null checks.  And address James' comments.
    
    clang-format
    
    (cherry picked from commit 784e7cc8a91b48b93b57cad27ba403684880969f)
---
 proxy/http/HttpSM.cc           | 47 ++++++++++++++++++++----------------------
 proxy/http/HttpServerSession.h |  1 +
 proxy/http/HttpTransact.cc     | 25 +++++++++++-----------
 3 files changed, 35 insertions(+), 38 deletions(-)

diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index c53ce2c..3f6d338 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -1640,8 +1640,8 @@ HttpSM::handle_api_return()
           DebugSM("http_websocket",
                   "(client session) Setting websocket active timeout=%" PRId64 
"s and inactive timeout=%" PRId64 "s",
                   t_state.txn_conf->websocket_active_timeout, 
t_state.txn_conf->websocket_inactive_timeout);
-          
ua_session->get_netvc()->set_active_timeout(HRTIME_SECONDS(t_state.txn_conf->websocket_active_timeout));
-          
ua_session->get_netvc()->set_inactivity_timeout(HRTIME_SECONDS(t_state.txn_conf->websocket_inactive_timeout));
+          
ua_session->set_active_timeout(HRTIME_SECONDS(t_state.txn_conf->websocket_active_timeout));
+          
ua_session->set_inactivity_timeout(HRTIME_SECONDS(t_state.txn_conf->websocket_inactive_timeout));
         }
 
         if (server_session) {
@@ -2112,14 +2112,17 @@ HttpSM::process_hostdb_info(HostDBInfo *r)
   bool use_client_addr        = 
t_state.http_config_param->use_client_target_addr == 1 && 
t_state.client_info.is_transparent &&
                          t_state.dns_info.os_addr_style == 
HttpTransact::DNSLookupInfo::OS_ADDR_TRY_DEFAULT;
   if (use_client_addr) {
-    client_addr = 
t_state.state_machine->ua_session->get_netvc()->get_local_addr();
-    // Regardless of whether the client address matches the DNS record or not,
-    // we want to use that address.  Therefore, we copy over the client address
-    // info and skip the assignment from the DNS cache
-    ats_ip_copy(t_state.host_db_info.ip(), client_addr);
-    t_state.dns_info.os_addr_style  = 
HttpTransact::DNSLookupInfo::OS_ADDR_TRY_CLIENT;
-    t_state.dns_info.lookup_success = true;
-    // Leave ret unassigned, so we don't overwrite the host_db_info
+    NetVConnection *vc = t_state.state_machine->ua_session ? 
t_state.state_machine->ua_session->get_netvc() : NULL;
+    if (vc) {
+      client_addr = vc->get_local_addr();
+      // Regardless of whether the client address matches the DNS record or 
not,
+      // we want to use that address.  Therefore, we copy over the client 
address
+      // info and skip the assignment from the DNS cache
+      ats_ip_copy(t_state.host_db_info.ip(), client_addr);
+      t_state.dns_info.os_addr_style  = 
HttpTransact::DNSLookupInfo::OS_ADDR_TRY_CLIENT;
+      t_state.dns_info.lookup_success = true;
+      // Leave ret unassigned, so we don't overwrite the host_db_info
+    }
   }
 
   if (r && !r->is_failed()) {
@@ -5806,12 +5809,6 @@ HttpSM::attach_server_session(HttpServerSession *s)
     return;
   }
 
-  if (ua_session) {
-    NetVConnection *server_vc = s->get_netvc();
-    NetVConnection *ua_vc     = ua_session->get_netvc();
-    ink_release_assert(server_vc->thread == ua_vc->thread);
-  }
-
   // Set the mutex so that we have something to update
   //   stats with
   server_session->mutex = this->mutex;
@@ -5830,7 +5827,10 @@ HttpSM::attach_server_session(HttpServerSession *s)
   UnixNetVConnection *server_vc = dynamic_cast<UnixNetVConnection 
*>(server_session->get_netvc());
   UnixNetVConnection *client_vc = (UnixNetVConnection 
*)(ua_session->get_netvc());
   SSLNetVConnection *ssl_vc     = dynamic_cast<SSLNetVConnection *>(client_vc);
-  bool associated_connection    = false;
+
+  // Verifying that the user agent and server sessions/transactions are 
operating on the same thread.
+  ink_release_assert(!server_vc || !client_vc || server_vc->thread == 
client_vc->thread);
+  bool associated_connection = false;
   if (server_vc) { // if server_vc isn't a PluginVC
     if (ssl_vc) {  // if incoming connection is SSL
       bool client_trace = ssl_vc->getSSLTrace();
@@ -7231,7 +7231,10 @@ HttpSM::set_next_state()
     } else if (t_state.dns_info.looking_up == HttpTransact::ORIGIN_SERVER && 
t_state.http_config_param->no_dns_forward_to_parent &&
                t_state.parent_result.result != PARENT_UNDEFINED) {
       if (t_state.cop_test_page) {
-        ats_ip_copy(t_state.host_db_info.ip(), 
t_state.state_machine->ua_session->get_netvc()->get_local_addr());
+        NetVConnection *vc = t_state.state_machine->ua_session->get_netvc();
+        if (vc) {
+          ats_ip_copy(t_state.host_db_info.ip(), vc->get_local_addr());
+        }
       }
 
       t_state.dns_info.lookup_success = true;
@@ -7286,13 +7289,7 @@ HttpSM::set_next_state()
       // sending its request and for this reason, the inactivity timeout
       // cannot be cancelled.
       if (ua_session && !t_state.hdr_info.request_content_length) {
-        NetVConnection *vc = ua_session->get_netvc();
-        if (vc) {
-          ua_session->cancel_inactivity_timeout();
-        } else {
-          terminate_sm = true;
-          return; // Give up if there is no session netvc
-        }
+        ua_session->cancel_inactivity_timeout();
       } else if (!ua_session) {
         terminate_sm = true;
         return; // Give up if there is no session
diff --git a/proxy/http/HttpServerSession.h b/proxy/http/HttpServerSession.h
index 5a39ad1..5400d4e 100644
--- a/proxy/http/HttpServerSession.h
+++ b/proxy/http/HttpServerSession.h
@@ -136,6 +136,7 @@ public:
     ink_release_assert(server_vc != NULL);
     return server_vc->get_remote_endpoint();
   }
+
   INK_MD5 hostname_hash;
 
   int64_t con_id;
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index e252778..1146ad7 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -568,7 +568,8 @@ HttpTransact::BadRequest(State *s)
 void
 HttpTransact::HandleBlindTunnel(State *s)
 {
-  bool inbound_transparent_p = 
s->state_machine->ua_session->get_netvc()->get_is_transparent();
+  NetVConnection *vc         = s->state_machine->ua_session->get_netvc();
+  bool inbound_transparent_p = vc->get_is_transparent();
   URL u;
   // IpEndpoint dest_addr;
   // ip_text_buffer new_host;
@@ -590,10 +591,10 @@ HttpTransact::HandleBlindTunnel(State *s)
   s->hdr_info.client_request.version_set(ver);
 
   char new_host[INET6_ADDRSTRLEN];
-  ats_ip_ntop(s->state_machine->ua_session->get_netvc()->get_local_addr(), 
new_host, sizeof(new_host));
+  ats_ip_ntop(vc->get_local_addr(), new_host, sizeof(new_host));
 
   s->hdr_info.client_request.url_get()->host_set(new_host, strlen(new_host));
-  
s->hdr_info.client_request.url_get()->port_set(s->state_machine->ua_session->get_netvc()->get_local_port());
+  s->hdr_info.client_request.url_get()->port_set(vc->get_local_port());
 
   // Initialize the state vars necessary to sending error responses
   bootstrap_state_variables_from_request(s, &s->hdr_info.client_request);
@@ -935,7 +936,7 @@ HttpTransact::EndRemapRequest(State *s)
 done:
   // We now set the active-timeout again, since it might have been changed as 
part of the remap rules.
   if (s->state_machine->ua_session) {
-    
s->state_machine->ua_session->get_netvc()->set_active_timeout(HRTIME_SECONDS(s->txn_conf->transaction_active_timeout_in));
+    
s->state_machine->ua_session->set_active_timeout(HRTIME_SECONDS(s->txn_conf->transaction_active_timeout_in));
   }
 
   if (is_debug_tag_set("http_chdr_describe") || is_debug_tag_set("http_trans") 
|| is_debug_tag_set("url_rewrite")) {
@@ -5705,14 +5706,15 @@ 
HttpTransact::initialize_state_variables_from_request(State *s, HTTPHdr *obsolet
     s->client_info.proxy_connect_hdr = true;
   }
 
-  if (s->state_machine->ua_session) {
-    s->request_data.incoming_port = 
s->state_machine->ua_session->get_netvc()->get_local_port();
-    s->request_data.internal_txn  = 
s->state_machine->ua_session->get_netvc()->get_is_internal_request();
-  }
   NetVConnection *vc = NULL;
   if (s->state_machine->ua_session) {
     vc = s->state_machine->ua_session->get_netvc();
   }
+
+  if (vc) {
+    s->request_data.incoming_port = vc->get_local_port();
+    s->request_data.internal_txn  = vc->get_is_internal_request();
+  }
   // If this is an internal request, never keep alive
   if (!s->txn_conf->keep_alive_enabled_in || (vc && 
vc->get_is_internal_request()) ||
       (s->state_machine->ua_session && 
s->state_machine->ua_session->ignore_keep_alive())) {
@@ -5795,11 +5797,8 @@ 
HttpTransact::initialize_state_variables_from_request(State *s, HTTPHdr *obsolet
   s->request_data.hostname_str = s->arena.str_store(host_name, host_len);
   ats_ip_copy(&s->request_data.src_ip, &s->client_info.src_addr);
   memset(&s->request_data.dest_ip, 0, sizeof(s->request_data.dest_ip));
-  if (s->state_machine->ua_session) {
-    NetVConnection *netvc = s->state_machine->ua_session->get_netvc();
-    if (netvc) {
-      s->request_data.incoming_port = netvc->get_local_port();
-    }
+  if (vc) {
+    s->request_data.incoming_port = vc->get_local_port();
   }
   s->request_data.xact_start = s->client_request_time;
   s->request_data.api_info   = &s->api_info;

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <commits@trafficserver.apache.org>'].

Reply via email to