From 1171fbf892891c0c8ad9ad3b724c2ee9ea18ce93 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 2 Sep 2026 11:58:51 +0200 Subject: [PATCH] Extract a reusable connect poll helper and enforce bounds at the write --- .../api/api_outgoing_connection.cpp | 46 ++++++++++++------- .../components/api/api_outgoing_connection.h | 12 +++++ esphome/components/api/api_server.cpp | 12 ++++- esphome/components/api/api_server.h | 3 +- .../components/socket/bsd_sockets_impl.cpp | 6 ++- .../components/socket/lwip_sockets_impl.cpp | 6 ++- 6 files changed, 62 insertions(+), 23 deletions(-) diff --git a/esphome/components/api/api_outgoing_connection.cpp b/esphome/components/api/api_outgoing_connection.cpp index 5727e57389..f1d25ade8f 100644 --- a/esphome/components/api/api_outgoing_connection.cpp +++ b/esphome/components/api/api_outgoing_connection.cpp @@ -88,6 +88,9 @@ void OutgoingConnectionManager::try_dial_(APIServer *server, uint32_t now) { // (covers an IPv6 literal left by an earlier enable_ipv6 build too) this->saved_ = {}; this->host_persisted_ = this->target_pref_.save(&this->saved_) && global_preferences->sync(); + if (!this->host_persisted_) { + ESP_LOGW(TAG, "Failed to clear target"); + } #endif this->schedule_retry_(now); return; @@ -125,12 +128,26 @@ void OutgoingConnectionManager::poll_connect_(APIServer *server, uint32_t now) { return; } this->last_poll_ = now; - int fd = this->dial_socket_->get_fd(); + int err = 0; + switch (poll_connect(*this->dial_socket_, err)) { + case ConnectPollResult::CONNECT_POLL_PENDING: + break; + case ConnectPollResult::CONNECT_POLL_CONNECTED: + this->handoff_(server, now); + break; + case ConnectPollResult::CONNECT_POLL_ERROR: + ESP_LOGW(TAG, "Connect failed: %d", err); + this->schedule_retry_(now); + break; + } +} + +ConnectPollResult poll_connect(socket::Socket &sock, int &err_out) { + int fd = sock.get_fd(); if (fd < 0 || fd >= FD_SETSIZE) { // FD_SET on either is undefined behavior - ESP_LOGW(TAG, "fd %d unusable for select", fd); - this->schedule_retry_(now); - return; + err_out = EBADF; + return ConnectPollResult::CONNECT_POLL_ERROR; } // Connect completion is a write event; the main loop only selects on reads fd_set writefds; @@ -145,26 +162,23 @@ void OutgoingConnectionManager::poll_connect_(APIServer *server, uint32_t now) { int ret = ::select(fd + 1, nullptr, &writefds, nullptr, &tv); #endif if (ret < 0) { - ESP_LOGW(TAG, "Connect poll failed: errno %d", errno); - this->schedule_retry_(now); - return; + err_out = errno; + return ConnectPollResult::CONNECT_POLL_ERROR; } if (ret == 0 || !FD_ISSET(fd, &writefds)) { - return; // still in progress + return ConnectPollResult::CONNECT_POLL_PENDING; } int error = 0; socklen_t len = sizeof(error); - if (this->dial_socket_->getsockopt(SOL_SOCKET, SO_ERROR, &error, &len) != 0) { - ESP_LOGW(TAG, "Connect status check failed: errno %d", errno); - this->schedule_retry_(now); - return; + if (sock.getsockopt(SOL_SOCKET, SO_ERROR, &error, &len) != 0) { + err_out = errno; + return ConnectPollResult::CONNECT_POLL_ERROR; } if (error != 0) { - ESP_LOGW(TAG, "Connect failed: %d", error); - this->schedule_retry_(now); - return; + err_out = error; + return ConnectPollResult::CONNECT_POLL_ERROR; } - this->handoff_(server, now); + return ConnectPollResult::CONNECT_POLL_CONNECTED; } void OutgoingConnectionManager::handoff_(APIServer *server, uint32_t now) { diff --git a/esphome/components/api/api_outgoing_connection.h b/esphome/components/api/api_outgoing_connection.h index 781955cc05..a48ab961d8 100644 --- a/esphome/components/api/api_outgoing_connection.h +++ b/esphome/components/api/api_outgoing_connection.h @@ -25,6 +25,18 @@ class APIConnection; // target is simply relearned static constexpr size_t SAVED_TARGET_HOST_LEN = socket::SOCKADDR_STR_LEN; +enum class ConnectPollResult : uint8_t { + CONNECT_POLL_PENDING, + CONNECT_POLL_CONNECTED, + CONNECT_POLL_ERROR, +}; + +/// Check a non-blocking connect() for completion without blocking. On +/// CONNECT_POLL_ERROR, err_out holds the socket's SO_ERROR, or errno when the +/// poll itself failed. Lives here for now; a candidate for the socket +/// component (async_tcp has a near-duplicate poll). +ConnectPollResult poll_connect(socket::Socket &sock, int &err_out); + struct SavedOutgoingTarget { // IP as text so the socket component's v4-mapped-IPv6 normalization is // reused on both ends; empty = none remembered diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index de5b050e32..6e97e2a2f6 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -282,7 +282,12 @@ void __attribute__((flatten)) APIServer::accept_new_connections_() { } } -void APIServer::add_client_(APIConnection *conn) { +bool APIServer::add_client_(APIConnection *conn) { + if (this->at_client_limit_()) { + // Callers check first; enforce the array bound where the write happens + delete conn; + return false; + } this->clients_[this->api_connection_count_++].reset(conn); conn->start(); @@ -292,6 +297,7 @@ void APIServer::add_client_(APIConnection *conn) { if (this->api_connection_count_ == 1 && this->reboot_timeout_ != 0 && !this->provisioning_pending_()) { this->status_clear_warning(); } + return true; } #ifdef USE_API_OUTGOING_CONNECTION @@ -304,7 +310,9 @@ APIConnection *APIServer::add_outgoing_client_(std::unique_ptr s return nullptr; } auto *conn = new APIConnection(std::move(sock), this); - this->add_client_(conn); + if (!this->add_client_(conn)) { + return nullptr; + } // After start(): sends our server hello first so the peer can pick the key conn->mark_outgoing(); return conn; diff --git a/esphome/components/api/api_server.h b/esphome/components/api/api_server.h index e61645758c..b50134c93e 100644 --- a/esphome/components/api/api_server.h +++ b/esphome/components/api/api_server.h @@ -264,7 +264,8 @@ class APIServer final : public Component, // Accept incoming socket connections. Only called when socket has pending connections. void __attribute__((noinline)) accept_new_connections_(); // Insert a constructed connection into the client slots and start it. - void add_client_(APIConnection *conn); + // Takes ownership; deletes the connection and returns false at the limit + bool add_client_(APIConnection *conn); bool at_client_limit_() const { return this->api_connection_count_ >= MAX_API_CONNECTIONS; } #ifdef USE_API_OUTGOING_CONNECTION // Returns the new connection, or nullptr (socket dropped) when at the limit diff --git a/esphome/components/socket/bsd_sockets_impl.cpp b/esphome/components/socket/bsd_sockets_impl.cpp index 0d4284f145..cec3b3498a 100644 --- a/esphome/components/socket/bsd_sockets_impl.cpp +++ b/esphome/components/socket/bsd_sockets_impl.cpp @@ -59,13 +59,15 @@ int BSDSocketImpl::close() { int BSDSocketImpl::setblocking(bool blocking) { int fl = ::fcntl(this->fd_, F_GETFL, 0); + if (fl < 0) { + return fl; + } if (blocking) { fl &= ~O_NONBLOCK; } else { fl |= O_NONBLOCK; } - ::fcntl(this->fd_, F_SETFL, fl); - return 0; + return ::fcntl(this->fd_, F_SETFL, fl); } size_t BSDSocketImpl::getpeername_to(std::span buf) { diff --git a/esphome/components/socket/lwip_sockets_impl.cpp b/esphome/components/socket/lwip_sockets_impl.cpp index a6bd639c10..f87c5377e4 100644 --- a/esphome/components/socket/lwip_sockets_impl.cpp +++ b/esphome/components/socket/lwip_sockets_impl.cpp @@ -49,13 +49,15 @@ int LwIPSocketImpl::close() { int LwIPSocketImpl::setblocking(bool blocking) { int fl = lwip_fcntl(this->fd_, F_GETFL, 0); + if (fl < 0) { + return fl; + } if (blocking) { fl &= ~O_NONBLOCK; } else { fl |= O_NONBLOCK; } - lwip_fcntl(this->fd_, F_SETFL, fl); - return 0; + return lwip_fcntl(this->fd_, F_SETFL, fl); } size_t LwIPSocketImpl::getpeername_to(std::span buf) {