[PR #5214] [MERGED] fix: concurrent map crash, error shadowing, and unchecked ListenUDP error #5179

New issue

Closed

opened 2026-05-05 14:56:13 -06:00 by gitea-mirror · 0 comments

gitea-mirror commented

2026-05-05 14:56:13 -06:00

Owner

📋 Pull Request Information

Original PR: https://github.com/fatedier/frp/pull/5214
Author: @fatedier
Created: 3/7/2026
Status: ✅ Merged
Merged: 3/7/2026
Merged by: @fatedier

Base: dev ← Head: new

📝 Commits (1)

cbe65d8 fix: three high-severity bugs across nathole, proxy, and udp modules

📊 Changes

3 files changed (+8 additions, -1 deletions)

View changed files

📝 pkg/nathole/controller.go (+2 -0)
📝 pkg/util/net/udp.go (+4 -0)
📝 server/proxy/proxy.go (+2 -1)

📄 Description

Summary

Fixes three high-severity bugs found via static analysis and cross-verified by Codex:

pkg/nathole: concurrent map read/write crash — HandleVisitor PreCheck path reads c.clientCfgs without lock while ListenClient/CloseClient modify it under c.mu. Since the handler runs via msg.AsyncHandler (goroutine), this is a data race that can crash the server with concurrent map read and map write. Fix: add c.mu.RLock() around the map read.
server/proxy: closed connection returned with nil error — GetWorkConnFromPool uses err := msg.WriteMsg(...) (short variable declaration) which shadows the outer named return err. When poolCount is 0 and WriteMsg fails, the outer err stays nil, causing the function to return a closed workConn with no error. Fix: use err = instead of err :=, and set workConn = nil after closing.
pkg/util/net: nil pointer panic on ListenUDP failure — net.ListenUDP error was not checked before spawning goroutines that call readConn.ReadFromUDP. If listen fails, readConn is nil → panic. Also l.readConn was never assigned, making Close() a no-op. Fix: check error and return early, assign readConn to struct field.

Test plan

make build passes
make test passes
Codex review — no issues found

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/fatedier/frp/pull/5214 **Author:** [@fatedier](https://github.com/fatedier) **Created:** 3/7/2026 **Status:** ✅ Merged **Merged:** 3/7/2026 **Merged by:** [@fatedier](https://github.com/fatedier) **Base:** `dev` ← **Head:** `new` --- ### 📝 Commits (1) - [`cbe65d8`](https://github.com/fatedier/frp/commit/cbe65d855bfa0abd985c3c27a4587f39034ee645) fix: three high-severity bugs across nathole, proxy, and udp modules ### 📊 Changes **3 files changed** (+8 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `pkg/nathole/controller.go` (+2 -0) 📝 `pkg/util/net/udp.go` (+4 -0) 📝 `server/proxy/proxy.go` (+2 -1) </details> ### 📄 Description ## Summary Fixes three high-severity bugs found via static analysis and cross-verified by Codex: - **pkg/nathole: concurrent map read/write crash** — `HandleVisitor` PreCheck path reads `c.clientCfgs` without lock while `ListenClient`/`CloseClient` modify it under `c.mu`. Since the handler runs via `msg.AsyncHandler` (goroutine), this is a data race that can crash the server with `concurrent map read and map write`. Fix: add `c.mu.RLock()` around the map read. - **server/proxy: closed connection returned with nil error** — `GetWorkConnFromPool` uses `err := msg.WriteMsg(...)` (short variable declaration) which shadows the outer named return `err`. When `poolCount` is 0 and `WriteMsg` fails, the outer `err` stays nil, causing the function to return a closed `workConn` with no error. Fix: use `err =` instead of `err :=`, and set `workConn = nil` after closing. - **pkg/util/net: nil pointer panic on ListenUDP failure** — `net.ListenUDP` error was not checked before spawning goroutines that call `readConn.ReadFromUDP`. If listen fails, `readConn` is nil → panic. Also `l.readConn` was never assigned, making `Close()` a no-op. Fix: check error and return early, assign `readConn` to struct field. ## Test plan - [x] `make build` passes - [x] `make test` passes - [x] Codex review — no issues found --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>