Skip to content
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

Fix SEGFAULT errors introduced by dns_resolver in socketio (gh# 1275) #415

Merged
merged 6 commits into from
Nov 22, 2019
Merged

Fix SEGFAULT errors introduced by dns_resolver in socketio (gh# 1275) #415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
da5a58b
Fix SEGFAULT errors introduced by dns_resolver in socketio (gh# 1275)
ewertons Nov 21, 2019
25cc4e8
Merge branch 'master' into FixDnsREsolverSegfault
ewertons Nov 21, 2019
f94ffdb
[ACR] Fix SEGFAULT errors introduced by dns_resolver in socketio (gh#…
ewertons Nov 21, 2019
eabaf0b
Merge branch 'FixDnsREsolverSegfault' of https://github.com/Azure/azu…
ewertons Nov 21, 2019
fb594cf
Cleanup socketio_create and socketio_destroy functions
ewertons Nov 21, 2019
40a4e53
Update unit tests for fixes in socketio due to dns_resolver bug
ewertons Nov 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 41 additions & 14 deletions adapters/socketio_berkeley.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,18 @@ static int initiate_socket_connection(SOCKET_IO_INSTANCE* socket_io_instance)
else
{
addr = dns_resolver_get_addrInfo(socket_io_instance->dns_resolver);
connect_addr = addr->ai_addr;
connect_addr_len = sizeof(*addr->ai_addr);
result = 0;

if (addr == NULL)
{
LogError("DNS resolution failed");
result = MU_FAILURE;
}
else
{
connect_addr = addr->ai_addr;
connect_addr_len = sizeof(*addr->ai_addr);
result = 0;
}
}
}
else
Expand Down Expand Up @@ -644,6 +653,24 @@ static int set_target_network_interface(int socket, char* mac_address)
}
#endif //__APPLE__

static void destroy_socket_io_instance(SOCKET_IO_INSTANCE* instance)
{
if (instance->dns_resolver != NULL)
{
dns_resolver_destroy(instance->dns_resolver);
}

free(instance->hostname);
free(instance->target_mac_address);

if (instance->pending_io_list != NULL)
{
singlylinkedlist_destroy(instance->pending_io_list);
}

free(instance);
}

CONCRETE_IO_HANDLE socketio_create(void* io_create_parameters)
{
SOCKETIO_CONFIG* socket_io_config = io_create_parameters;
Expand All @@ -659,12 +686,14 @@ CONCRETE_IO_HANDLE socketio_create(void* io_create_parameters)
result = malloc(sizeof(SOCKET_IO_INSTANCE));
if (result != NULL)
{
(void)memset(result, 0, sizeof(SOCKET_IO_INSTANCE));

result->address_type = ADDRESS_TYPE_IP;
result->pending_io_list = singlylinkedlist_create();
if (result->pending_io_list == NULL)
{
LogError("Failure: singlylinkedlist_create unable to create pending list.");
free(result);
destroy_socket_io_instance(result);
result = NULL;
}
else
Expand All @@ -688,15 +717,19 @@ CONCRETE_IO_HANDLE socketio_create(void* io_create_parameters)
if ((result->hostname == NULL) && (result->socket == INVALID_SOCKET))
{
LogError("Failure: hostname == NULL and socket is invalid.");
singlylinkedlist_destroy(result->pending_io_list);
free(result);
destroy_socket_io_instance(result);
result = NULL;
}
else if ((result->dns_resolver = dns_resolver_create(result->hostname, socket_io_config->port, NULL)) == NULL)
{
LogError("Failure creating dns_resolver");
destroy_socket_io_instance(result);
result = NULL;
}
else
{
result->port = socket_io_config->port;
result->on_io_open_complete = NULL;
result->dns_resolver = dns_resolver_create(result->hostname, result->port, NULL);
result->target_mac_address = NULL;
result->on_bytes_received = NULL;
result->on_io_error = NULL;
Expand Down Expand Up @@ -741,13 +774,7 @@ void socketio_destroy(CONCRETE_IO_HANDLE socket_io)
first_pending_io = singlylinkedlist_get_head_item(socket_io_instance->pending_io_list);
}

singlylinkedlist_destroy(socket_io_instance->pending_io_list);
free(socket_io_instance->hostname);
free(socket_io_instance->target_mac_address);

dns_resolver_destroy(socket_io_instance->dns_resolver);

free(socket_io);
destroy_socket_io_instance(socket_io_instance);
}
}

Expand Down
109 changes: 66 additions & 43 deletions adapters/socketio_win32.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,21 +212,29 @@ static int initiate_socket_connection(SOCKET_IO_INSTANCE* socket_io_instance)
{
#endif
struct addrinfo* addr = dns_resolver_get_addrInfo(socket_io_instance->dns_resolver);
(void)memcpy((socket_io_instance->addrInfo), addr, sizeof(*(socket_io_instance->addrInfo)));

result = connect_socket(socket_io_instance->socket, (socket_io_instance->addrInfo)->ai_addr, sizeof(*((socket_io_instance->addrInfo)->ai_addr)));
if(result != 0)
if (addr == NULL)
{
LogError("connect_socket failed");
LogError("DNS resolution failed");
result = MU_FAILURE;
}
else
else
{
if (socket_io_instance->on_io_open_complete != NULL)
(void)memcpy((socket_io_instance->addrInfo), addr, sizeof(*(socket_io_instance->addrInfo)));

result = connect_socket(socket_io_instance->socket, (socket_io_instance->addrInfo)->ai_addr, sizeof(*((socket_io_instance->addrInfo)->ai_addr)));
if (result != 0)
{
socket_io_instance->on_io_open_complete(socket_io_instance->on_io_open_complete_context, IO_OPEN_OK /*: IO_OPEN_ERROR*/);
LogError("connect_socket failed");
}
else
{
if (socket_io_instance->on_io_open_complete != NULL)
{
socket_io_instance->on_io_open_complete(socket_io_instance->on_io_open_complete_context, IO_OPEN_OK /*: IO_OPEN_ERROR*/);
}
}
}

#ifdef AF_UNIX_ON_WINDOWS
}
else // ADDRESS_TYPE_DOMAIN_SOCKET
Expand Down Expand Up @@ -279,6 +287,24 @@ static int initiate_socket_connection(SOCKET_IO_INSTANCE* socket_io_instance)
return result;
}

static void destroy_socket_io_instance(SOCKET_IO_INSTANCE* instance)
{
if (instance->dns_resolver != NULL)
{
dns_resolver_destroy(instance->dns_resolver);
}

free(instance->hostname);
free(instance->addrInfo);

if (instance->pending_io_list != NULL)
{
singlylinkedlist_destroy(instance->pending_io_list);
}

free(instance);
}

CONCRETE_IO_HANDLE socketio_create(void* io_create_parameters)
{
SOCKETIO_CONFIG* socket_io_config = (SOCKETIO_CONFIG*)io_create_parameters;
Expand All @@ -293,14 +319,17 @@ CONCRETE_IO_HANDLE socketio_create(void* io_create_parameters)
else
{
result = (SOCKET_IO_INSTANCE*)malloc(sizeof(SOCKET_IO_INSTANCE));

if (result != NULL)
{
(void)memset(result, 0, sizeof(SOCKET_IO_INSTANCE));

result->address_type = ADDRESS_TYPE_IP;
result->pending_io_list = singlylinkedlist_create();
if (result->pending_io_list == NULL)
{
LogError("Failure: singlylinkedlist_create unable to create pending list.");
free(result);
destroy_socket_io_instance(result);
result = NULL;
}
else
Expand All @@ -324,35 +353,37 @@ CONCRETE_IO_HANDLE socketio_create(void* io_create_parameters)
if ((result->hostname == NULL) && (result->socket == INVALID_SOCKET))
{
LogError("Failure: hostname == NULL and socket is invalid.");
singlylinkedlist_destroy(result->pending_io_list);
free(result);
destroy_socket_io_instance(result);
result = NULL;
}
else if ((result->addrInfo = calloc(1, sizeof(struct addrinfo))) == NULL)
{
LogError("Failure: addrInfo == NULL.");
destroy_socket_io_instance(result);
result = NULL;
}
else if ((result->addrInfo->ai_addr = calloc(1, sizeof(struct sockaddr_in))) == NULL)
{
LogError("Failure allocating ai_addr");
destroy_socket_io_instance(result);
result = NULL;
}
else if ((result->dns_resolver = dns_resolver_create(result->hostname, socket_io_config->port, NULL)) == NULL)
{
LogError("Failure creating dns_resolver");
destroy_socket_io_instance(result);
result = NULL;
}
else
{
result->addrInfo = calloc(1, sizeof(struct addrinfo));
if (result->addrInfo == NULL)
{
LogError("Failure: addrInfo == NULL.");
singlylinkedlist_destroy(result->pending_io_list);
free(result);
result = NULL;
}
else
{
result->addrInfo->ai_addr = calloc(1, sizeof(struct sockaddr_in));

result->port = socket_io_config->port;
result->on_io_open_complete = NULL;
result->dns_resolver = dns_resolver_create(result->hostname, result->port, NULL);
result->on_bytes_received = NULL;
result->on_io_error = NULL;
result->on_bytes_received_context = NULL;
result->on_io_error_context = NULL;
result->io_state = IO_STATE_CLOSED;
result->keep_alive = tcp_keepalive;
}

result->port = socket_io_config->port;
result->on_io_open_complete = NULL;
result->on_bytes_received = NULL;
result->on_io_error = NULL;
result->on_bytes_received_context = NULL;
result->on_io_error_context = NULL;
result->io_state = IO_STATE_CLOSED;
result->keep_alive = tcp_keepalive;
}
}
}
Expand Down Expand Up @@ -389,15 +420,7 @@ void socketio_destroy(CONCRETE_IO_HANDLE socket_io)
singlylinkedlist_remove(socket_io_instance->pending_io_list, first_pending_io);
}

singlylinkedlist_destroy(socket_io_instance->pending_io_list);
if (socket_io_instance->hostname != NULL)
{
free(socket_io_instance->hostname);
}

dns_resolver_destroy(socket_io_instance->dns_resolver);

free(socket_io);
destroy_socket_io_instance(socket_io);
}
}

Expand Down
7 changes: 5 additions & 2 deletions tests/socketio_win32_ut/socketio_win32_ut.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,8 @@ TEST_FUNCTION(socketio_create_singlylinkedlist_create_fails)
EXPECTED_CALL(gballoc_malloc(IGNORED_NUM_ARG));
EXPECTED_CALL(singlylinkedlist_create()).SetReturn((SINGLYLINKEDLIST_HANDLE)NULL);
EXPECTED_CALL(gballoc_free(IGNORED_PTR_ARG));
EXPECTED_CALL(gballoc_free(IGNORED_PTR_ARG));
EXPECTED_CALL(gballoc_free(IGNORED_PTR_ARG));

// act
ioHandle = socketio_create(&socketConfig);
Expand Down Expand Up @@ -532,12 +534,13 @@ TEST_FUNCTION(socketio_destroy_socket_succeeds)
EXPECTED_CALL(gballoc_free(IGNORED_PTR_ARG));
EXPECTED_CALL(singlylinkedlist_remove(IGNORED_PTR_ARG, IGNORED_PTR_ARG));
EXPECTED_CALL(singlylinkedlist_get_head_item(IGNORED_PTR_ARG));
EXPECTED_CALL(singlylinkedlist_destroy(IGNORED_PTR_ARG));
EXPECTED_CALL(gballoc_free(IGNORED_PTR_ARG));
EXPECTED_CALL(freeaddrinfo(&TEST_ADDR_INFO));
EXPECTED_CALL(gballoc_free(IGNORED_PTR_ARG));
EXPECTED_CALL(gballoc_free(IGNORED_PTR_ARG));
EXPECTED_CALL(gballoc_free(IGNORED_PTR_ARG));
EXPECTED_CALL(gballoc_free(IGNORED_PTR_ARG));
EXPECTED_CALL(singlylinkedlist_destroy(IGNORED_PTR_ARG));
EXPECTED_CALL(gballoc_free(IGNORED_PTR_ARG));

// act
socketio_destroy(ioHandle);
Expand Down