Merge pull request #14740 from chrisswinchatt-arm/fix-netsocket-dynalloc

Fix 'netsocket: several dynamic allocation results not checked' (#14210)

add_event_listener in NetworkInterface now returns an error if the method fails. Previous attempts to add the event listener would attempt to use an unchecked standard dynamically allocated ns_list_* item.

In other cases, the dynamically allocated items will now be checked, and if unsuccessful, will return after cleaning up any outstanding issues.

TCPSocket::accept will now check that its own internally allocated new TCPSocket call will succeed, and if not, will clean up the stack resources. This should help when memory is low but an incoming connection requests a connection when the TCPSocket is listening.

Author: 0xc0170

connectivity/netsocket/include/netsocket/NetworkInterface.h

@@ -377,6 +377,18 @@ class NetworkInterface: public DNS {
      */
     virtual void attach(mbed::Callback<void(nsapi_event_t, intptr_t)> status_cb);
 
+#if MBED_CONF_NSAPI_ADD_EVENT_LISTENER_RETURN_CHANGE
+    /** Add event listener for interface.
+     *
+     * This API allows multiple callback to be registered for a single interface.
+     * of both leads to undefined behavior.
+     *
+     *  @param status_cb The callback for status changes.
+     *  @return NSAPI_ERROR_OK on success
+     *  @return NSAPI_ERROR_NO_MEMORY if the function fails to create a new entry.
+     */
+    nsapi_error_t add_event_listener(mbed::Callback<void(nsapi_event_t, intptr_t)> status_cb);
+#else
     /** Add event listener for interface.
      *
      * This API allows multiple callback to be registered for a single interface.
@@ -386,9 +398,16 @@ class NetworkInterface: public DNS {
      * Application may only use attach() or add_event_listener() interface. Mixing usage
      * of both leads to undefined behavior.
      *
+     * @warning This version of the function does not use the `std::nothrow` feature. Subsequently,
+     * the function may fail to allocate memory and cause a system error. To use the new
+     * version with the changes, set "nsapi.add-event-listener-return-change": 1 in the
+     * target overrides section in your mbed_app.json file.
+     *
      *  @param status_cb The callback for status changes.
      */
+    MBED_DEPRECATED_SINCE("mbed-os-6.12", "This function return value will change to nsapi_error_t in the next major release. See documentation for details.")
     void add_event_listener(mbed::Callback<void(nsapi_event_t, intptr_t)> status_cb);
+#endif
 
 #if MBED_CONF_PLATFORM_CALLBACK_COMPARABLE
     /** Remove event listener from interface.
@@ -512,6 +531,10 @@ class NetworkInterface: public DNS {
      * configuration).
      */
     virtual void set_default_parameters();
+
+private:
+    // Unified implementation for different versions of add_event_listener.
+    nsapi_error_t internal_add_event_listener(mbed::Callback<void(nsapi_event_t, intptr_t)> status_cb);
 };
 
 #endif

connectivity/netsocket/mbed_lib.json

@@ -2,6 +2,10 @@
     "name": "nsapi",
     "config": {
         "present": 1,
+        "add-event-listener-return-change": {
+            "help": "Updates the add_event_listener to return a nsapi_error_t value which can indicate allocation failure. See documents for more details.",
+            "value": 0
+        },
         "default-stack": {
             "help" : "Default stack to be used, valid values: LWIP, NANOSTACK.",
             "value" : "LWIP"

connectivity/netsocket/source/NetworkInterface.cpp

@@ -20,6 +20,7 @@
 #include "netsocket/NetworkStack.h"
 #include "platform/Callback.h"
 #include "platform/mbed_error.h"
+#include <new>
 #include <string.h>
 
 // Default network-interface state
@@ -142,22 +143,48 @@ static void call_all_event_listeners(NetworkInterface *iface, nsapi_event_t even
     }
 }
 
-void NetworkInterface::add_event_listener(mbed::Callback<void(nsapi_event_t, intptr_t)> status_cb)
+nsapi_error_t NetworkInterface::internal_add_event_listener(mbed::Callback<void(nsapi_event_t, intptr_t)> status_cb)
 {
     iface_eventlist_t *event_list = get_interface_event_list_head();
 #if MBED_CONF_PLATFORM_CALLBACK_COMPARABLE
     ns_list_foreach_safe(iface_eventlist_entry_t, entry, event_list) {
         if (entry->status_cb == status_cb && entry->iface == this) {
-            return;
+            return NSAPI_ERROR_OK;
         }
     }
 #endif
-    iface_eventlist_entry_t *entry = new iface_eventlist_entry_t;
+
+    iface_eventlist_entry_t *entry = new (std::nothrow) iface_eventlist_entry_t;
+    if (!entry) {
+        return NSAPI_ERROR_NO_MEMORY;
+    }
+
     entry->iface = this;
     entry->status_cb = status_cb;
     ns_list_add_to_end(event_list, entry);
     attach(mbed::callback(&call_all_event_listeners, this));
+    return NSAPI_ERROR_OK;
+}
+
+#if MBED_CONF_NSAPI_ADD_EVENT_LISTENER_RETURN_CHANGE
+nsapi_error_t NetworkInterface::add_event_listener(mbed::Callback<void(nsapi_event_t, intptr_t)> status_cb)
+{
+    return internal_add_event_listener(status_cb);
+}
+#else
+void NetworkInterface::add_event_listener(mbed::Callback<void(nsapi_event_t, intptr_t)> status_cb)
+{
+    auto error = internal_add_event_listener(status_cb);
+    switch (error) {
+        case NSAPI_ERROR_OK:
+            return;
+        case NSAPI_ERROR_NO_MEMORY:
+            MBED_ERROR(error, "Out of memory when adding an event listener");
+        default:
+            MBED_UNREACHABLE;
+    }
 }
+#endif // MBED_CONF_NSAPI_ADD_EVENT_LISTENER_RETURN_CHANGE
 
 #if MBED_CONF_PLATFORM_CALLBACK_COMPARABLE
 void NetworkInterface::remove_event_listener(mbed::Callback<void(nsapi_event_t, intptr_t)> status_cb)

connectivity/netsocket/source/TCPSocket.cpp

@@ -16,6 +16,7 @@
  */
 
 #include "netsocket/TCPSocket.h"
+#include <new>
 #include "Timer.h"
 #include "mbed_assert.h"
 
@@ -266,7 +267,12 @@ TCPSocket *TCPSocket::accept(nsapi_error_t *error)
         ret = _stack->socket_accept(_socket, &socket, &address);
 
         if (0 == ret) {
-            connection = new TCPSocket(this, socket, address);
+            connection = new (std::nothrow) TCPSocket(this, socket, address);
+            if (!connection) {
+                ret = NSAPI_ERROR_NO_MEMORY;
+                break;
+            }
+
             _socket_stats.stats_update_peer(connection, address);
             _socket_stats.stats_update_socket_state(connection, SOCK_CONNECTED);
             break;

connectivity/netsocket/source/TLSSocketWrapper.cpp

@@ -16,6 +16,7 @@
  */
 
 #include "netsocket/TLSSocketWrapper.h"
+#include <new>
 #include "platform/Callback.h"
 #include "drivers/Timer.h"
 #include "events/mbed_events.h"
@@ -134,7 +135,10 @@ nsapi_error_t TLSSocketWrapper::set_client_cert_key(const void *client_cert, siz
 #else
 
     int ret;
-    mbedtls_x509_crt *crt = new mbedtls_x509_crt;
+    mbedtls_x509_crt *crt = new (std::nothrow) mbedtls_x509_crt;
+    if (!crt) {
+        return NSAPI_ERROR_NO_MEMORY;
+    }
     mbedtls_x509_crt_init(crt);
     if ((ret = mbedtls_x509_crt_parse(crt, static_cast<const unsigned char *>(client_cert),
                                       client_cert_len)) != 0) {
@@ -286,7 +290,11 @@ nsapi_error_t TLSSocketWrapper::continue_handshake()
 #if defined(MBEDTLS_X509_CRT_PARSE_C) && defined(FEA_TRACE_SUPPORT) && !defined(MBEDTLS_X509_REMOVE_INFO)
     /* Prints the server certificate and verify it. */
     const size_t buf_size = 1024;
-    char *buf = new char[buf_size];
+    char *buf = new (std::nothrow) char[buf_size];
+    if (!buf) {
+        print_mbedtls_error("new (std::nothrow) char[buf_size] failed in continue_handshake", NSAPI_ERROR_NO_MEMORY);
+        return NSAPI_ERROR_NO_MEMORY;
+    }
     mbedtls_x509_crt_info(buf, buf_size, "\r    ",
                           mbedtls_ssl_get_peer_cert(&_ssl));
     tr_debug("Server certificate:\r\n%s\r\n", buf);
@@ -427,10 +435,9 @@ void TLSSocketWrapper::print_mbedtls_error(MBED_UNUSED const char *name, MBED_UN
 {
 // Avoid pulling in mbedtls_strerror when trace is not enabled
 #if defined FEA_TRACE_SUPPORT && defined MBEDTLS_ERROR_C
-    char *buf = new char[128];
+    char buf[128];
     mbedtls_strerror(err, buf, 128);
     tr_err("%s() failed: -0x%04x (%d): %s", name, -err, err, buf);
-    delete[] buf;
 #else
     tr_err("%s() failed: -0x%04x (%d)", name, -err, err);
 #endif
@@ -569,7 +576,10 @@ mbedtls_ssl_config *TLSSocketWrapper::get_ssl_config()
 {
     if (!_ssl_conf) {
         int ret;
-        _ssl_conf = new mbedtls_ssl_config;
+        _ssl_conf = new (std::nothrow) mbedtls_ssl_config;
+        if (!_ssl_conf) {
+            return nullptr;
+        }
         mbedtls_ssl_config_init(_ssl_conf);
         _ssl_conf_allocated = true;
 

connectivity/netsocket/source/nsapi_dns.cpp

@@ -982,7 +982,12 @@ static void nsapi_dns_query_async_create(void *ptr)
         }
 
         if (!query->socket_cb_data) {
-            query->socket_cb_data = new SOCKET_CB_DATA;
+            query->socket_cb_data = new (std::nothrow) SOCKET_CB_DATA;
+            if (!query->socket_cb_data) {
+                delete socket;
+                nsapi_dns_query_async_resp(query, NSAPI_ERROR_NO_MEMORY, NULL);
+                return;
+            }
         }
         query->socket_cb_data->call_in_cb = query->call_in_cb;
         query->socket_cb_data->stack = query->stack;

tools/test/travis-ci/doxy-spellchecker/ignore.en.pws

@@ -121,3 +121,5 @@ instantiation
 instantiations
 KVStore
 _doxy_
+nothrow
+conf