You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bc...@apache.org on 2016/10/12 21:56:22 UTC
[trafficserver] branch 7.0.x updated: TS-4938: Avoid crashes due to
NULL vc dereferences.
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 <sh...@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" <co...@trafficserver.apache.org>'].