Merge pull request #4 from MaurUppi/codex/read-and-document-dns-audit-report

Fix DNS audit findings: DoUDP conn race, dialSend context/fallback, add race CI

Author: MaurUppi

.github/workflows/dns-race.yml

@@ -0,0 +1,31 @@
+name: DNS Race Test
+
+on:
+  pull_request:
+    paths:
+      - "control/**/*.go"
+      - "go.mod"
+      - "go.sum"
+      - ".github/workflows/dns-race.yml"
+  push:
+    branches:
+      - main
+    paths:
+      - "control/**/*.go"
+      - "go.mod"
+      - "go.sum"
+      - ".github/workflows/dns-race.yml"
+
+jobs:
+  race-test:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: '^1.22'
+
+      - name: Run race detector for control package
+        run: go test -race ./control/...

.plan/code_audit_report-dev.md

@@ -0,0 +1,40 @@
+# dae DNS 改进代码审计报告 - 开发执行文档
+
+> 来源：`.plan/code_audit_report.md`
+> 目标：按 `Removal/Iteration Plan` 严格串行落实变更、验证与记录。
+
+## 0. 执行总则（强制）
+1. 所有任务严格串行执行：`Tn` 未测试通过，不开始 `Tn+1`。
+2. 每个任务必须包含：代码实现、任务级测试、测试记录。
+3. 每个里程碑必须在全部任务通过后执行一次回归测试。
+4. 任一测试失败，先修复并重测，直至通过。
+
+## 1. 任务拆解（对应 Removal/Iteration Plan）
+
+### T1：删除 dead code（P1-1）
+- 目标：删除 `control/dns_control.go` 中无效分支 `if err != nil { return err }`。
+- 影响：仅清理控制流，不改变业务行为。
+- 验收：代码扫描确认该 dead code 不存在。
+
+### T2：修复 DoUDP 并发竞争（P0-1）
+- 目标：`DoUDP.ForwardDNS` 中 goroutine 与读取路径不再共享可变 `d.conn` 引用。
+- 实施：使用局部连接变量进行读写，避免复用实例时写错连接；重试计时改为 ticker。
+- 验收：代码扫描确认 goroutine/read 均使用局部连接变量。
+
+### T3：修复 fallback 错误返回语义（P1-2）
+- 目标：TCP fallback forwarder 创建失败时返回 `fallbackErr` 包装错误，而非原始 UDP 超时错误。
+- 验收：代码扫描确认返回错误包含 fallback 创建失败信息与原始错误上下文。
+
+### T4：dialSend context 传播（P1-3）
+- 目标：`dialSend` 不再使用 `context.TODO()`，改为接收上层 context 并在递归路径透传。
+- 验收：代码扫描确认 `context.WithTimeout(ctx, ...)`，且递归调用透传 `ctx`。
+
+### T5：CI 增加 race detector 检查
+- 目标：新增工作流执行 `go test -race ./control/...`。
+- 验收：工作流文件存在且命令准确。
+
+## 2. 里程碑回归
+- 命令：
+  - `go test ./control -run 'Test(IsTimeoutError|TcpFallbackDialArgument|SendStreamDNSRespectsContextCancelBeforeIO|EvictDnsForwarderCacheOneLocked)' -count=1`
+  - `go test -race ./control/...`
+- 说明：如受依赖拉取限制，需记录失败原因与影响范围到 `.plan/test-log.md`。

.plan/test-log.md

@@ -67,3 +67,36 @@
 - 命令：`go test ./control -run 'Test(IsTimeoutError|TcpFallbackDialArgument|SendStreamDNSRespectsContextCancelBeforeIO|EvictDnsForwarderCacheOneLocked)' -count=1`
 - 结果：失败（环境限制），`proxy.golang.org` 拉取私有/受限依赖 `github.com/daeuniverse/outbound` 返回 403 Forbidden。
 - 结论：在当前环境无法完成自动化回归编译；已保留任务级静态校验记录。
+
+## Code Audit Iteration - T1（移除 dead code）
+- 命令：`sed -n '626,650p' control/dns_control.go`
+- 结果：`forwarder.ForwardDNS(ctxDial, data)` 前不再存在 `if err != nil { return err }` 的残留分支。
+- 结论：通过（dead code 已移除）。
+
+## Code Audit Iteration - T2（DoUDP 并发竞争修复）
+- 命令：`sed -n '312,360p' control/dns.go`
+- 结果：`DoUDP.ForwardDNS` 新增 `localConn := conn`，goroutine 写入与主流程读取均使用 `localConn`，重试等待改为 `retryTicker`。
+- 结论：通过（避免 goroutine 与后续调用共享可变 `d.conn`）。
+
+## Code Audit Iteration - T3（fallback 错误语义修复）
+- 命令：`rg -n "tcp fallback forwarder creation failed" control/dns_control.go`
+- 结果：命中 `return fmt.Errorf("tcp fallback forwarder creation failed: %w (original: %v)", fallbackErr, err)`。
+- 结论：通过（fallback 创建失败不再误报为原始 UDP 错误）。
+
+## Code Audit Iteration - T4（dialSend context 传播）
+- 命令：`rg -n "dialSend\(context.Background\(|func \(c \*DnsController\) dialSend\(ctx context.Context|context.WithTimeout\(ctx, consts.DefaultDialTimeout\)|dialSend\(ctx, invokingDepth\+1" control/dns_control.go`
+- 结果：命中入口传入 `context.Background()`、`dialSend(ctx ...)` 签名、`WithTimeout(ctx, ...)`、递归透传 `ctx`。
+- 结论：通过（已去除 `context.TODO()`）。
+
+## Code Audit Iteration - T5（CI race detector）
+- 命令：`rg -n "go test -race ./control/..." .github/workflows/dns-race.yml`
+- 结果：命中新增工作流中的 race 检测命令。
+- 结论：通过（CI 已补充 race 检测入口）。
+
+## Code Audit Iteration - 里程碑回归
+- 命令：`go test ./control -run 'Test(IsTimeoutError|TcpFallbackDialArgument|SendStreamDNSRespectsContextCancelBeforeIO|EvictDnsForwarderCacheOneLocked)' -count=1`
+- 结果：失败，依赖 `github.com/daeuniverse/outbound` 从 `proxy.golang.org` 拉取返回 403 Forbidden。
+- 结论：受环境限制，无法完成自动化回归编译。
+- 命令：`go test -race ./control/...`
+- 结果：失败，除上述依赖拉取 403 外，`control/kern/tests` 还出现 `bpftestObjects/loadBpftestObjects` 未定义构建错误。
+- 结论：受环境限制，未能在本地完成 race 回归；已在 CI 增加对应检测工作流。

control/dns.go

@@ -314,16 +314,19 @@ func (d *DoUDP) ForwardDNS(ctx context.Context, data []byte) (*dnsmessage.Msg, e
 		return nil, err
 	}
 	d.conn = conn
+	localConn := conn
 
 	timeout := 5 * time.Second
-	_ = d.conn.SetDeadline(time.Now().Add(timeout))
+	_ = localConn.SetDeadline(time.Now().Add(timeout))
 	dnsReqCtx, cancelDnsReqCtx := context.WithTimeout(ctx, timeout)
 	defer cancelDnsReqCtx()
+	retryTicker := time.NewTicker(1 * time.Second)
+	defer retryTicker.Stop()
 
 	go func() {
 		// Send DNS request every seconds.
 		for {
-			_, _ = d.conn.Write(data)
+			_, _ = localConn.Write(data)
 			// if err != nil {
 			// 	if c.log.IsLevelEnabled(logrus.DebugLevel) {
 			// 		c.log.WithFields(logrus.Fields{
@@ -341,7 +344,7 @@ func (d *DoUDP) ForwardDNS(ctx context.Context, data []byte) (*dnsmessage.Msg, e
 			select {
 			case <-dnsReqCtx.Done():
 				return
-			case <-time.After(1 * time.Second):
+			case <-retryTicker.C:
 			}
 		}
 	}()
@@ -350,7 +353,7 @@ func (d *DoUDP) ForwardDNS(ctx context.Context, data []byte) (*dnsmessage.Msg, e
 	respBuf := pool.GetFullCap(consts.EthernetMtu)
 	defer pool.Put(respBuf)
 	// Wait for response.
-	n, err := d.conn.Read(respBuf)
+	n, err := localConn.Read(respBuf)
 	if err != nil {
 		return nil, err
 	}

control/dns_control.go

@@ -536,7 +536,7 @@ func (c *DnsController) handle_(
 	if err != nil {
 		return fmt.Errorf("pack DNS packet: %w", err)
 	}
-	return c.dialSend(0, req, data, dnsMessage.Id, upstream, needResp)
+	return c.dialSend(context.Background(), 0, req, data, dnsMessage.Id, upstream, needResp)
 }
 
 // sendReject_ send empty answer.
@@ -562,7 +562,7 @@ func (c *DnsController) sendReject_(dnsMessage *dnsmessage.Msg, req *udpRequest)
 	return nil
 }
 
-func (c *DnsController) dialSend(invokingDepth int, req *udpRequest, data []byte, id uint16, upstream *dns.Upstream, needResp bool) (err error) {
+func (c *DnsController) dialSend(ctx context.Context, invokingDepth int, req *udpRequest, data []byte, id uint16, upstream *dns.Upstream, needResp bool) (err error) {
 	if invokingDepth >= MaxDnsLookupDepth {
 		return fmt.Errorf("too deep DNS lookup invoking (depth: %v); there may be infinite loop in your DNS response routing", MaxDnsLookupDepth)
 	}
@@ -607,7 +607,7 @@ func (c *DnsController) dialSend(invokingDepth int, req *udpRequest, data []byte
 	// We should set a connClosed flag to avoid it.
 	var connClosed bool
 
-	ctxDial, cancel := context.WithTimeout(context.TODO(), consts.DefaultDialTimeout)
+	ctxDial, cancel := context.WithTimeout(ctx, consts.DefaultDialTimeout)
 	defer cancel()
 
 	// get forwarder from cache
@@ -632,10 +632,6 @@ func (c *DnsController) dialSend(invokingDepth int, req *udpRequest, data []byte
 		}
 	}()
 
-	if err != nil {
-		return err
-	}
-
 	respMsg, err = forwarder.ForwardDNS(ctxDial, data)
 	if err != nil {
 		if c.timeoutExceedCallback != nil && isTimeoutError(err) {
@@ -644,7 +640,7 @@ func (c *DnsController) dialSend(invokingDepth int, req *udpRequest, data []byte
 		if fallbackDialArgument := tcpFallbackDialArgument(upstream, dialArgument, err); fallbackDialArgument != nil {
 			fallbackForwarder, fallbackErr := newDnsForwarder(upstream, *fallbackDialArgument)
 			if fallbackErr != nil {
-				return err
+				return fmt.Errorf("tcp fallback forwarder creation failed: %w (original: %v)", fallbackErr, err)
 			}
 			defer fallbackForwarder.Close()
 			respMsg, fallbackErr = fallbackForwarder.ForwardDNS(ctxDial, data)
@@ -702,7 +698,7 @@ func (c *DnsController) dialSend(invokingDepth int, req *udpRequest, data []byte
 				"next_upstream": nextUpstream.String(),
 			}).Traceln("Change DNS upstream and resend")
 		}
-		return c.dialSend(invokingDepth+1, req, data, id, nextUpstream, needResp)
+		return c.dialSend(ctx, invokingDepth+1, req, data, id, nextUpstream, needResp)
 	}
 	if upstreamIndex.IsReserved() && c.log.IsLevelEnabled(logrus.InfoLevel) {
 		var (