From 87c053888417cd583f1be518cab23de0b3275f25 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 10 Apr 2026 21:27:23 -1000 Subject: [PATCH] [socket] Address Copilot review: tighten doc comments - cached_sock_ comment in both impl headers: drop the 'iff' wording since the pointer can also be null if esphome_lwip_get_sock() fails on a fd that was requested to be monitored. Document all three null cases explicitly, plus the close()-path nulling for UAF protection. - application.h register_socket_fd / unregister_socket_fd comment block: move inside the #ifdef USE_HOST so the generic 'register/unregister a socket' wording no longer implies these APIs exist on fast-select builds. Add a forward reference to fast_select_hook_fd for readers wondering where the ESP32/LibreTiny equivalent went. --- esphome/components/socket/bsd_sockets_impl.h | 10 +++++++--- esphome/components/socket/lwip_sockets_impl.h | 10 +++++++--- esphome/core/application.h | 8 +++++--- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/esphome/components/socket/bsd_sockets_impl.h b/esphome/components/socket/bsd_sockets_impl.h index 64b6adf94a8..57c1a430a2b 100644 --- a/esphome/components/socket/bsd_sockets_impl.h +++ b/esphome/components/socket/bsd_sockets_impl.h @@ -124,9 +124,13 @@ class BSDSocketImpl { // destructor / double-close path just check fd_ < 0. int fd_{-1}; #ifdef USE_LWIP_FAST_SELECT - // Non-null iff this socket is being monitored for read events. Replaces loop_monitored_ - // on the fast-select path: the pointer itself carries the "monitored" bit. - struct lwip_sock *cached_sock_{nullptr}; // Cached for direct rcvevent read in ready() + // Cached lwip_sock pointer used for direct rcvevent reads in ready() on the + // fast-select path. Replaces loop_monitored_: null means this socket is not being + // monitored for read events — either monitoring was not requested, the fd was + // invalid, or esphome_lwip_get_sock() failed. Non-null means the netconn event + // callback was hooked and notifications are flowing. close() nulls this to prevent + // use-after-free via a recycled lwip slot. + struct lwip_sock *cached_sock_{nullptr}; #else bool loop_monitored_{false}; #endif diff --git a/esphome/components/socket/lwip_sockets_impl.h b/esphome/components/socket/lwip_sockets_impl.h index 6e275eb6d17..7f3b706cd8d 100644 --- a/esphome/components/socket/lwip_sockets_impl.h +++ b/esphome/components/socket/lwip_sockets_impl.h @@ -90,9 +90,13 @@ class LwIPSocketImpl { // destructor / double-close path just check fd_ < 0. int fd_{-1}; #ifdef USE_LWIP_FAST_SELECT - // Non-null iff this socket is being monitored for read events. Replaces loop_monitored_ - // on the fast-select path: the pointer itself carries the "monitored" bit. - struct lwip_sock *cached_sock_{nullptr}; // Cached for direct rcvevent read in ready() + // Cached lwip_sock pointer used for direct rcvevent reads in ready() on the + // fast-select path. Replaces loop_monitored_: null means this socket is not being + // monitored for read events — either monitoring was not requested, the fd was + // invalid, or esphome_lwip_get_sock() failed. Non-null means the netconn event + // callback was hooked and notifications are flowing. close() nulls this to prevent + // use-after-free via a recycled lwip slot. + struct lwip_sock *cached_sock_{nullptr}; #else bool loop_monitored_{false}; #endif diff --git a/esphome/core/application.h b/esphome/core/application.h index 47976fd57b1..8c9fff2b53a 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -532,11 +532,13 @@ class Application { Scheduler scheduler; - /// Register/unregister a socket to be monitored for read events. - /// WARNING: These functions are NOT thread-safe. They must only be called from the main loop. #ifdef USE_HOST - /// Fallback select() path: monitors file descriptors. + /// Register/unregister a socket file descriptor with the host select() fallback loop. + /// USE_LWIP_FAST_SELECT builds do not use this API — sockets hook the lwIP netconn + /// event_callback directly (see socket.h fast_select_hook_fd) and rely on FreeRTOS + /// task notifications for wake-up. /// NOTE: File descriptors >= FD_SETSIZE (typically 10 on ESP) will be rejected with an error. + /// WARNING: These functions are NOT thread-safe. They must only be called from the main loop. /// @return true if registration was successful, false if fd exceeds limits bool register_socket_fd(int fd); void unregister_socket_fd(int fd);