From 7fd801fe451db14679b7f20f76a3e942081a1ce4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 14 Mar 2026 16:55:01 -1000 Subject: [PATCH 1/2] [ethernet] Fix lwIP locking races in RP2040 ethernet - Copy dns_getserver() values (not pointers) under LwIPLock in dump_connect_params_() to avoid dereferencing stale pointers - Read netif ip_addr/netmask/gw under LwIPLock to prevent races with DHCP callbacks from IRQ context - Remove duplicate connect_begin_ assignment in start_connect_() - Remove duplicate status_set_warning() call in start_connect_() --- .../ethernet/ethernet_component_rp2040.cpp | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/esphome/components/ethernet/ethernet_component_rp2040.cpp b/esphome/components/ethernet/ethernet_component_rp2040.cpp index 22a748ef48a..354901b5823 100644 --- a/esphome/components/ethernet/ethernet_component_rp2040.cpp +++ b/esphome/components/ethernet/ethernet_component_rp2040.cpp @@ -250,9 +250,6 @@ void EthernetComponent::start_connect_() { } } #endif - - this->connect_begin_ = millis(); - this->status_set_warning(); } void EthernetComponent::finish_connect_() { @@ -272,13 +269,16 @@ void EthernetComponent::dump_connect_params_() { char dns2_buf[network::IP_ADDRESS_BUFFER_SIZE]; char mac_buf[MAC_ADDRESS_PRETTY_BUFFER_SIZE]; - auto *netif = this->eth_->getNetIf(); - const ip_addr_t *dns_ip1; - const ip_addr_t *dns_ip2; + // Copy all lwIP state under the lock to avoid races with IRQ callbacks + ip_addr_t ip_addr, netmask, gw, dns1_addr, dns2_addr; { LwIPLock lock; - dns_ip1 = dns_getserver(0); - dns_ip2 = dns_getserver(1); + auto *netif = this->eth_->getNetIf(); + ip_addr = netif->ip_addr; + netmask = netif->netmask; + gw = netif->gw; + dns1_addr = *dns_getserver(0); + dns2_addr = *dns_getserver(1); } ESP_LOGCONFIG(TAG, " IP Address: %s\n" @@ -288,10 +288,10 @@ void EthernetComponent::dump_connect_params_() { " DNS1: %s\n" " DNS2: %s\n" " MAC Address: %s", - network::IPAddress(&netif->ip_addr).str_to(ip_buf), App.get_name().c_str(), - network::IPAddress(&netif->netmask).str_to(subnet_buf), - network::IPAddress(&netif->gw).str_to(gateway_buf), network::IPAddress(dns_ip1).str_to(dns1_buf), - network::IPAddress(dns_ip2).str_to(dns2_buf), this->get_eth_mac_address_pretty_into_buffer(mac_buf)); + network::IPAddress(&ip_addr).str_to(ip_buf), App.get_name().c_str(), + network::IPAddress(&netmask).str_to(subnet_buf), network::IPAddress(&gw).str_to(gateway_buf), + network::IPAddress(&dns1_addr).str_to(dns1_buf), network::IPAddress(&dns2_addr).str_to(dns2_buf), + this->get_eth_mac_address_pretty_into_buffer(mac_buf)); } void EthernetComponent::set_clk_pin(uint8_t clk_pin) { this->clk_pin_ = clk_pin; } From f109473707a7a78c3b48dd32bb1d8aa77b1f8057 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 14 Mar 2026 16:57:00 -1000 Subject: [PATCH 2/2] [ethernet] Document lock-free polling rationale in RP2040 loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Explain why linkStatus() and connected() are called without LwIPLock — linkStatus() only reads the W5500 PHY via SPI, and connected() does a single atomic 32-bit read of netif->ip_addr. --- esphome/components/ethernet/ethernet_component_rp2040.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/esphome/components/ethernet/ethernet_component_rp2040.cpp b/esphome/components/ethernet/ethernet_component_rp2040.cpp index 354901b5823..fd2ff4a6473 100644 --- a/esphome/components/ethernet/ethernet_component_rp2040.cpp +++ b/esphome/components/ethernet/ethernet_component_rp2040.cpp @@ -80,7 +80,11 @@ void EthernetComponent::setup() { } void EthernetComponent::loop() { - // On RP2040, we need to poll connection state since there are no events + // On RP2040, we need to poll connection state since there are no events. + // linkStatus() reads the W5500 PHY register via SPI — no lwip state involved. + // connected() reads netif->ip_addr without LwIPLock, but this is a single + // 32-bit aligned read (atomic on ARM) — worst case is a one-iteration-stale + // value, which is benign for polling. if (this->eth_ != nullptr) { bool link_up = this->eth_->linkStatus() == LinkON; bool has_ip = this->eth_->connected();