From b556488208e1adc643c78211026b6fa3bd0a66e1 Mon Sep 17 00:00:00 2001 From: Philip Fisher Date: Sat, 2 May 2020 14:54:41 -0400 Subject: [PATCH] Fix faulty io net error handling code in posix BSP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Basically, when the SDK allocates the structure in which it will store the socket descriptor it opens when connecting to the iotcore server, it initializes it to 0. If during the process of creating the socket, the posix BSP routine (iotc_bsp_io_net_socket_connect) that creates the socket can’t resolve the iotcore server name, it returns an error (leaving the socket descriptor field initialized to 0). In turn, the io net routine (iotc_io_net_layer_connect) that itself called iotc_bsp_io_net_socket_connect, on seeing the error, will then execute its error handling code that itself ends up calling the io net routine iotc_io_net_layer_close_externally that then calls the posix BSP iotc_bsp_io_net_close_socket routine. The posix BSP iotc_bsp_io_net_close_socket routine then blindly closes socket descriptor 0 (since this is what the data structure tracking the socket descriptor is still initialized to since no socket was ever successfully opened). Files modified: src/bsp/platform/posix/iotc_bsp_io_net_posix.c - always set *iotc_socket to -1 before attempting to connect or after closing the socket and always check that *iotc_socket is not -1 before attempting to close it in iotc_bsp_io_net_close_socket(). --- src/bsp/platform/posix/iotc_bsp_io_net_posix.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/bsp/platform/posix/iotc_bsp_io_net_posix.c b/src/bsp/platform/posix/iotc_bsp_io_net_posix.c index ae155130..3eac601c 100644 --- a/src/bsp/platform/posix/iotc_bsp_io_net_posix.c +++ b/src/bsp/platform/posix/iotc_bsp_io_net_posix.c @@ -49,6 +49,13 @@ iotc_bsp_io_net_state_t iotc_bsp_io_net_socket_connect( hints.ai_flags = 0; hints.ai_protocol = 0; + // in case of error, we want *iotc_socket initialized to a non-valid + // value so that iotc_bsp_io_net_close_socket can check before + // closing a socket to ensure that the socket was genuinely opened; + // otherwise iotc_bsp_io_net_close_socket can be at risk of + // incorrectly closing STDIN_FILENO + *iotc_socket = -1; + // Address resolution. status = getaddrinfo(host, NULL, &hints, &result); if (0 != status) { @@ -90,6 +97,7 @@ iotc_bsp_io_net_state_t iotc_bsp_io_net_socket_connect( return IOTC_BSP_IO_NET_STATE_OK; } else { close(*iotc_socket); + *iotc_socket = -1; } } } @@ -200,11 +208,16 @@ iotc_bsp_io_net_state_t iotc_bsp_io_net_close_socket( return IOTC_BSP_IO_NET_STATE_ERROR; } + if (*iotc_socket < 0) + // no need to close a socket that was never opened in the first + // place + return IOTC_BSP_IO_NET_STATE_OK; + shutdown(*iotc_socket, SHUT_RDWR); close(*iotc_socket); - *iotc_socket = 0; + *iotc_socket = -1; return IOTC_BSP_IO_NET_STATE_OK; }