757 lines
34 KiB
Markdown
757 lines
34 KiB
Markdown
# TQ_catN 工程深度代码审查报告(第二版)
|
||
|
||
**审查范围**:control_plane / domain / platform / app_composition 全部源文件
|
||
**审查维度**:内存安全、线程安全、状态机、资源管理、FreeRTOS 集成、架构合规
|
||
|
||
---
|
||
|
||
## 一、严重(Critical)— 可能导致崩溃或永久功能丧失
|
||
|
||
### C1. Watchdog 任务自删除导致多模态客户端永久失效
|
||
- **文件**:`components/control_plane/src/multimodal/control_plane_multimodal_runtime.c`
|
||
- **函数**:`multimodal_watchdog_task()` / `stop_watchdog_task()`
|
||
- **行号**:356-373, 375-435
|
||
|
||
```c
|
||
// watchdog 任务检测到空闲超时后:
|
||
control_plane_multimodal_stop(); // 内部调用 stop_watchdog_task()
|
||
|
||
// stop_watchdog_task:
|
||
if (!wait_task_exit(..., 1200)) {
|
||
TaskHandle_t task = get_multimodal_task_handle(&s_multimodal_watchdog_task);
|
||
if (task != NULL) {
|
||
vTaskDelete(task); // 删除自己,控制流不再返回
|
||
}
|
||
set_multimodal_task_handle(&s_multimodal_watchdog_task, NULL); // 永远执行不到
|
||
}
|
||
```
|
||
|
||
**问题**:watchdog 任务自己调用 `vTaskDelete(task)`(task 即自身),后续清理代码(将句柄置 NULL、重置 stop flag)永远执行不到。`s_multimodal_watchdog_task` 保持为悬空句柄,`s_multimodal_watchdog_stop_requested` 永久为 true。后续 `control_plane_start_multimodal_client()` 因句柄非 NULL 而拒绝重新创建 watchdog,**多模态客户端再也无法恢复**。
|
||
|
||
**修复**:watchdog 任务不应自删除。应通过 `xTaskNotifyGive` 通知外部任务执行删除,或使用 `vTaskDelete(NULL)` 后将清理工作移交给调用者。
|
||
|
||
---
|
||
|
||
### C2. Key Input ISR 对已释放队列的 Use-After-Free
|
||
- **文件**:`components/control_plane/src/runtime/control_plane_key_input.c`
|
||
- **函数**:`control_plane_key_gpio_isr_handler()` / `control_plane_stop_key_input_task()`
|
||
- **行号**:160-178, 493-515
|
||
|
||
```c
|
||
// ISR 中:
|
||
QueueHandle_t queue = s_gpio_irq_queue;
|
||
if (queue == NULL) { return; }
|
||
(void)xQueueSendFromISR(queue, &event, &higher_priority_task_woken);
|
||
|
||
// 停止流程中:
|
||
vQueueDelete(s_gpio_irq_queue); // queue 被释放
|
||
s_gpio_irq_queue = NULL;
|
||
```
|
||
|
||
**问题**:`gpio_isr_handler_remove()` 与 `vQueueDelete()` 之间无法保证中断不再触发。ISR 先将 `s_gpio_irq_queue` 读到局部变量 `queue`,随后主任务释放并删除队列,ISR 仍可能向已释放的 Queue 发送事件,导致内存损坏或系统崩溃。
|
||
|
||
**修复**:删除 queue 前应先 `gpio_intr_disable()` 所有引脚并确认 ISR 不再执行,或使用引用计数保护 queue 生命周期。
|
||
|
||
---
|
||
|
||
### C3. 音频下游队列释放后使用(UAF)
|
||
- **文件**:`components/platform/src/multimodal/audio_pipeline/audio_pipeline_runtime_impl.c`
|
||
- **函数**:`multimodal_audio_handle_downstream_opus()`
|
||
- **行号**:2056-2088
|
||
|
||
```c
|
||
if (!lock_downstream_playback_queue(pdMS_TO_TICKS(20))) { ... }
|
||
QueueHandle_t queue = ctx->downstream_playback_queue;
|
||
unlock_downstream_playback_queue();
|
||
|
||
// 解锁后在另一个任务可能销毁 queue 的情况下继续使用
|
||
if (xQueueSend(queue, &packet, pdMS_TO_TICKS(20)) == pdTRUE) { ... }
|
||
```
|
||
|
||
**问题**:解锁后、`xQueueSend` 前,若 `stop_downstream_playback_task()` 删除了该 queue,`xQueueSend` 操作已释放内存,导致崩溃。
|
||
|
||
**修复**:`xQueueSend` 必须在持有 `s_downstream_playback_queue_mutex` 的临界区内完成。
|
||
|
||
---
|
||
|
||
### C4. 音频任务强制删除时 TaskHandle 失效(UAF)
|
||
- **文件**:`components/platform/src/multimodal/audio_pipeline/audio_pipeline_runtime_impl.c`
|
||
- **函数**:`multimodal_audio_deinit()`
|
||
- **行号**:929-942
|
||
|
||
```c
|
||
audio_snapshot_uplink_tasks(&s_audio, &capture_task, &send_task);
|
||
if (capture_task != NULL) {
|
||
delete_audio_task_with_caps(capture_task); // 可能已自删
|
||
audio_set_uplink_capture_task(&s_audio, NULL);
|
||
}
|
||
```
|
||
|
||
**问题**:`uplink_capture_task` / `uplink_send_task` 在退出前会自删。获取句柄后任务可能已完成并自删;随后 `delete_audio_task_with_caps(capture_task)` 对已释放/不存在的 TCB 操作,在 FreeRTOS 中极易触发 `abort()` 或链表损坏。
|
||
|
||
**修复**:使用任务通知 + 超时等待任务自行退出,禁止在外部持有句柄时强制删除。
|
||
|
||
---
|
||
|
||
### C5. 校准服务未持有旋转互斥锁 — 步进电机资源冲突
|
||
- **文件**:`components/domain/src/calibration/domain_calibration_service.c`
|
||
- **函数**:`domain_calibrate_with_timeout`
|
||
- **行号**:113-231
|
||
|
||
**问题**:校准全程持有 `s_state_mutex`,但**从未**获取 `s_rotation_mutex`。`domain_direction_service.c` 中的 `domain_rotate_to_angle_with_timeout` 则先获取 `s_rotation_mutex` 再获取 `s_state_mutex`。当校准时,其他任务可成功获取 `s_rotation_mutex`,然后在 `s_state_mutex` 上阻塞;一旦校准释放 `s_state_mutex`,该任务立即进入步进电机调用,与校准的下一步电机操作**并发冲突**,导致机械位置失步或硬件损坏。
|
||
|
||
**修复**:校准函数也应按相同顺序获取 `s_rotation_mutex` 和 `s_state_mutex`。
|
||
|
||
---
|
||
|
||
### C6. 角度归一化函数对无穷值死循环
|
||
- **文件**:`components/domain/src/direction/domain_angle_math.c`
|
||
- **函数**:`domain_angle_normalize`
|
||
- **行号**:9-18
|
||
|
||
```c
|
||
while (angle > 180.0f) {
|
||
angle -= 360.0f; // 对 +inf 永远为 true,死循环
|
||
}
|
||
while (angle <= -180.0f) {
|
||
angle += 360.0f; // 对 -inf 永远为 true,死循环
|
||
}
|
||
```
|
||
|
||
**问题**:若传入 `INFINITY`、`-INFINITY` 或 `NAN`,`while` 循环永不退出,导致看门狗复位或任务挂死。
|
||
|
||
**修复**:增加前置检查 `if (!isfinite(angle)) return 0.0f;` 或使用 `fmodf` 替代循环。
|
||
|
||
---
|
||
|
||
### C7. WS 发送锁释放后立即删除
|
||
- **文件**:`components/platform/src/multimodal/transport_ws/transport_ws_runtime_impl.c`
|
||
- **函数**:`multimodal_ws_stop_sync_until()`
|
||
- **行号**:417-423
|
||
|
||
```c
|
||
if (send_lock_held && s_ws_send_lock != NULL) {
|
||
xSemaphoreGive(s_ws_send_lock);
|
||
}
|
||
if (send_lock_held && s_ws_send_lock != NULL) {
|
||
vSemaphoreDelete(s_ws_send_lock); // 另一任务可能刚被唤醒并持有该锁
|
||
s_ws_send_lock = NULL;
|
||
}
|
||
```
|
||
|
||
**问题**:`xSemaphoreGive` 后、`vSemaphoreDelete` 前,其他正在等待 `s_ws_send_lock` 的任务可能获得该信号量;随后信号量被删除,导致等待任务使用已释放对象。
|
||
|
||
**修复**:在仍持有锁的状态下直接删除(或改用静态信号量/互斥量避免动态删除)。
|
||
|
||
---
|
||
|
||
### C8. 下游空闲静音 Timer 回调竞态删除
|
||
- **文件**:`components/platform/src/multimodal/audio_pipeline/audio_pipeline_runtime_impl.c`
|
||
- **函数**:`stop_downstream_idle_mute_task()` / `downstream_idle_mute_timer_cb()`
|
||
- **行号**:1479-1506, 1301-1316
|
||
|
||
```c
|
||
esp_timer_stop(timer);
|
||
esp_timer_delete(timer); // 若 timer 回调正在另一核执行,删除不安全
|
||
```
|
||
|
||
**问题**:`esp_timer_delete()` **不等待**正在执行的回调结束。若回调正在运行,`timer` 及其 `arg` 被释放后,回调中继续访问 `ctx` 导致 UAF。
|
||
|
||
**修复**:删除前设置原子标志让回调自行短路返回,并确保无活跃回调。
|
||
|
||
---
|
||
|
||
### C9. `multimodal_ws_is_connected()` 对已销毁 client 句柄操作
|
||
- **文件**:`components/platform/src/multimodal/transport_ws/transport_ws.c`
|
||
- **函数**:`multimodal_ws_is_connected()`
|
||
- **行号**:458-470
|
||
|
||
```c
|
||
ws_client_state_snapshot(&client, &connected, &stop_requested);
|
||
...
|
||
return esp_websocket_client_is_connected(client); // client 可能已被销毁
|
||
```
|
||
|
||
**问题**:退出临界区后调用 `esp_websocket_client_is_connected(client)`,在此期间 `multimodal_ws_stop_sync_until()` 可能已 `destroy(client)` 并将句柄置 NULL。对无效/已释放句柄调用 esp_websocket API 属于 UAF。
|
||
|
||
**修复**:将 `is_connected` 判断完全放在 `s_client_state_lock` 临界区内,或增加引用计数。
|
||
|
||
---
|
||
|
||
### C10. LAN HTTP Server 强制删除任务时 IO Buffer UAF
|
||
- **文件**:`components/platform/src/runtime/platform_lan_http_server.c`
|
||
- **函数**:`platform_lan_http_server_stop()` / `server_task()`
|
||
- **行号**:822-839, 594-727
|
||
|
||
**问题**:强制删除 `server_task` 后,立即调用 `server_release_io_buffers()` 释放 buffer。若被删任务恰好在 `handle_client()` 执行路径中(已读取到局部指针但尚未返回),将继续访问已释放内存。
|
||
|
||
**修复**:在停止任务前通过 `shutdown(s_client_fd, SHUT_RDWR)` 中断阻塞的 IO,等待任务自然退出;仅在超时后才强制删除,且删除后延迟释放 buffer。
|
||
|
||
---
|
||
|
||
## 二、高(High)— 可能导致竞态条件、数据损坏或功能异常
|
||
|
||
### H1. `s_started` 与 `s_transport_available` 不一致导致永久连接失败
|
||
- **文件**:`components/control_plane/src/multimodal/control_plane_multimodal_session_service.c`
|
||
- **行号**:1533-1576
|
||
- **问题**:`control_plane_multimodal_start()` 先设置 `s_started = true`,再启动 transport。如果 transport 启动后立即断连,`s_started` 仍为 true,下次调用 `start()` 时幂等检查会跳过重连。
|
||
- **修复**:`s_started` 应在 `TRANSPORT_CONNECTED` 事件回调时才设置。
|
||
|
||
---
|
||
|
||
### H2. Session Service 互斥锁创建非线程安全
|
||
- **文件**:`components/control_plane/src/multimodal/control_plane_multimodal_session_service.c`
|
||
- **函数**:`ensure_state_mutex()`
|
||
- **行号**:283-298
|
||
|
||
```c
|
||
if (s_state_mutex != NULL) { return true; }
|
||
s_state_mutex = xSemaphoreCreateMutex(); // 非原子
|
||
```
|
||
|
||
**问题**:若两个线程首次并发进入,可能创建两个 mutex,第二个覆盖第一个导致泄漏;若已有线程获取了第一个 mutex,后续代码使用第二个 mutex,导致对共享状态无保护。
|
||
|
||
**修复**:使用 `portENTER_CRITICAL` 保护创建路径,或采用静态互斥量。
|
||
|
||
---
|
||
|
||
### H3. Device Auth 状态加载失败后被忽略继续初始化
|
||
- **文件**:`components/control_plane/src/multimodal/control_plane_multimodal_device_auth.c`
|
||
- **函数**:`control_plane_multimodal_device_auth_init()`
|
||
- **行号**:1856-1888
|
||
|
||
```c
|
||
esp_err_t err = load_state_from_store_locked();
|
||
if (err != ESP_OK) {
|
||
ESP_LOGW(TAG, "Auth store state unavailable, continue with defaults");
|
||
}
|
||
// ... 继续初始化,标记 s_auth.initialized = true
|
||
```
|
||
|
||
**问题**:当 `load_state_from_store_locked()` 返回 `ESP_ERR_INVALID_SIZE`(持久化数据损坏)时,仅打印 warning 即继续。此时 `s_auth.jwt` 等状态可能包含损坏的残留值,后续 `jwt_still_valid_locked()` 可能基于损坏的过期时间做出错误判断。
|
||
|
||
**修复**:数据损坏时应清除状态为默认值,而非保留残留值。
|
||
|
||
---
|
||
|
||
### H4. PTT 状态机基于过时 Snapshot 的 TOCTOU 竞态
|
||
- **文件**:`components/control_plane/src/runtime/control_plane_runtime_workers.c`
|
||
- **函数**:`control_plane_handle_ptt_teardown()`
|
||
- **行号**:1193-1243
|
||
|
||
```c
|
||
ptt_runtime_t ptt = control_plane_ptt_snapshot();
|
||
if (ptt.state == PTT_STATE_TEARDOWN_LOCAL) {
|
||
control_plane_multimodal_stop_uplink();
|
||
control_plane_ptt_set_state(PTT_STATE_TEARDOWN_REMOTE, "uplink_stopped");
|
||
ptt = control_plane_ptt_snapshot();
|
||
}
|
||
```
|
||
|
||
**问题**:第一次 snapshot 后判断为 `TEARDOWN_LOCAL`,随即停止 uplink 并推进到 `TEARDOWN_REMOTE`。但若在此期间用户已释放 PTT,`control_plane_set_ptt_active(false)` 已将状态重置为 `IDLE`。随后本函数仍强行将其推进到 `TEARDOWN_REMOTE` 并发送 `StopSpeech`,导致客户端与服务端状态不一致。
|
||
|
||
**修复**:在状态推进前检查 `generation` 是否仍有效,或改用临界区保护整个 teardown 流程。
|
||
|
||
---
|
||
|
||
### H5. PTT foreground_attempts 计数与 Snapshot 不一致
|
||
- **文件**:`components/control_plane/src/runtime/control_plane_runtime_workers.c`
|
||
- **函数**:`control_plane_drive_ptt_state_machine()`
|
||
- **行号**:1288-1303
|
||
|
||
```c
|
||
bool started = control_plane_try_start_multimodal_from_state_machine(...);
|
||
portENTER_CRITICAL(&s_runtime_lock);
|
||
s_ptt_runtime.foreground_attempts++;
|
||
portEXIT_CRITICAL(&s_runtime_lock);
|
||
if (!started &&
|
||
ptt.foreground_attempts + 1U >= CONTROL_PLANE_PTT_FOREGROUND_RECONNECT_MAX_ATTEMPTS) {
|
||
control_plane_ptt_set_state(PTT_STATE_DENIED, ...);
|
||
}
|
||
```
|
||
|
||
**问题**:判断条件使用 snapshot 中的旧 `ptt.foreground_attempts`,而实际值已在临界区内递增。若在此期间有其他代码重置了 `foreground_attempts`,此处仍依据旧值做出 `DENIED` 决策。
|
||
|
||
**修复**:在临界区内同时读取和判断 `foreground_attempts`。
|
||
|
||
---
|
||
|
||
### H6. WS 分片重组全局缓冲区无互斥保护
|
||
- **文件**:`components/platform/src/multimodal/transport_ws/transport_ws_workers_impl.c`
|
||
- **函数**:`process_text_fragment()` / `process_binary_fragment()`
|
||
|
||
**问题**:`s_text_reassembly_buf` / `s_binary_reassembly_buf` 在 queue 创建失败的 fallback 路径下,`handle_ws_msg_direct()` 会在 WebSocket client 事件任务上下文中直接调用这些函数。同时 `multimodal_ws_stop()` 会在调用者任务中执行 `reset_*_reassembly()`。两者无互斥,构成数据竞态。
|
||
|
||
**修复**:对重组缓冲区操作加锁,或确保 fallback 路径也串行化到同一任务。
|
||
|
||
---
|
||
|
||
### H7. `wait_worker_task_exit()` 对全局句柄无原子/屏障读取
|
||
- **文件**:`components/platform/src/multimodal/transport_ws/transport_ws_workers_impl.c`
|
||
- **函数**:`wait_worker_task_exit()`
|
||
- **行号**:472-489
|
||
|
||
```c
|
||
while (*task_handle != NULL && (xTaskGetTickCount() - start_tick) < timeout_ticks) {
|
||
vTaskDelay(pdMS_TO_TICKS(20));
|
||
}
|
||
```
|
||
|
||
**问题**:`s_ws_dispatch_task` 由被等待任务在退出时置 NULL,但本函数直接解引用读取,无原子操作或内存屏障。在 ESP32 多核缓存下,可能永远读到旧值导致超时后误判为未退出,进而执行对已自删任务的二次删除。
|
||
|
||
**修复**:使用 `volatile TaskHandle_t` + 适当内存屏障,或通过通知机制同步。
|
||
|
||
---
|
||
|
||
### H8. LCD 渲染任务强制删除导致 Mutex 永久占用
|
||
- **文件**:`components/platform/src/lcd_direct.c`
|
||
- **函数**:`lcd_direct_stop_render_loop()`
|
||
- **行号**:691-738
|
||
|
||
```c
|
||
if (task != xTaskGetCurrentTaskHandle()) {
|
||
vTaskDelete(task); // 若 task 正持有 s_lock,Mutex 将永久锁定
|
||
...
|
||
}
|
||
```
|
||
|
||
**问题**:渲染循环中会 `xSemaphoreTake(s_lock, ...)`。若任务在持锁期间被强制删除,FreeRTOS 不会自动释放 Mutex,导致后续所有尝试获取 `s_lock` 的操作永远超时。
|
||
|
||
**修复**:避免 `vTaskDelete` 强制终止;使用通知 + 轮询标志让任务自行退出。
|
||
|
||
---
|
||
|
||
### H9. 步进电机旋转长期占用 CPU 不释放
|
||
- **文件**:`components/platform/src/platform_stepper_driver.c`
|
||
- **函数**:`rotate_with_accel()`
|
||
- **行号**:275-303
|
||
|
||
```c
|
||
for (int i = 0; i < steps; i++) {
|
||
stepper_advance_phase(clockwise);
|
||
precise_delay_us(current_delay_us); // busy-wait
|
||
}
|
||
```
|
||
|
||
**问题**:以最快 600 us/步、最大 330° ≈ 3784 步计算,最长可连续自旋约 **2.3 秒** 不执行 `taskYIELD`。在此期间其他任务(包括音频采集、WS 保活)被饿死,可能导致音频欠采样、WS 断开。
|
||
|
||
**修复**:在步进循环中每隔一定步数调用 `taskYIELD()`,或将步进控制移到硬件定时器/LEDC。
|
||
|
||
---
|
||
|
||
### H10. Factory CLI 强制删除已自行退出的任务
|
||
- **文件**:`components/control_plane/src/runtime/control_plane_factory_cli.c`
|
||
- **函数**:`control_plane_factory_cli_stop()`
|
||
- **行号**:526-550
|
||
|
||
**问题**:若 worker 在循环等待期间已自行调用 `vTaskDelete(NULL)`,主线程第二次检查时理论上应返回。但存在竞态窗口:worker 尚未执行 `vTaskDelete(NULL)` 时,主线程仍得到非 NULL 句柄,随后执行 `vTaskDelete(worker_task)` 操作一个已结束任务的 TCB,属于未定义行为。
|
||
|
||
**修复**:使用 `xTaskNotifyWait` 或事件组等待 worker 明确报告退出,而非轮询 + 强制删除。
|
||
|
||
---
|
||
|
||
### H11. System Event / Key Input 任务直接强制删除
|
||
- **文件**:`components/control_plane/src/runtime/control_plane_system_events.c`
|
||
- **函数**:`control_plane_stop_system_event_task()`
|
||
- **行号**:257-276
|
||
|
||
```c
|
||
if (s_system_event_task != NULL) {
|
||
vTaskDelete(s_system_event_task); // 任务可能正在持有锁或访问共享资源
|
||
s_system_event_task = NULL;
|
||
}
|
||
```
|
||
|
||
**问题**:两处均在超时后直接 `vTaskDelete`。FreeRTOS 不保证被删任务持有的互斥锁会被释放。若任务在 `xQueueReceive` 临界区或持有某把锁时被删,将导致后续死锁或资源泄漏。
|
||
|
||
**修复**:使用任务通知 + 超时等待任务自行退出。
|
||
|
||
---
|
||
|
||
### H12. 浮点精度导致方向查询失败
|
||
- **文件**:`components/domain/src/direction/domain_direction_catalog.c`
|
||
- **函数**:`domain_direction_catalog_find_by_angle`
|
||
- **行号**:149
|
||
|
||
```c
|
||
if (entry->angle == angle) { // 精确浮点比较
|
||
```
|
||
|
||
**问题**:`domain_get_current_direction_index()` 将经多次浮点运算后的 `s_current_angle` 传入此函数。浮点运算会引入微小误差(如 `44.999996f` 与 `45.0f`),导致精确相等比较失败。
|
||
|
||
**修复**:引入 epsilon 容差比较,如 `fabsf(entry->angle - angle) < 0.5f`。
|
||
|
||
---
|
||
|
||
### H13. HTTP 响应 JSON 打印失败时缓冲区残留脏数据
|
||
- **文件**:`components/domain/src/fortune/domain_user_config_server.c`
|
||
- **函数**:`response_json_object`
|
||
- **行号**:130-136
|
||
|
||
```c
|
||
if (!cJSON_PrintPreallocated(root, (char *)response->body,
|
||
(int)response->body_capacity, false)) {
|
||
response->body_len = 0U;
|
||
return ESP_ERR_INVALID_SIZE;
|
||
}
|
||
```
|
||
|
||
**问题**:`cJSON_PrintPreallocated` 在缓冲区不足时可能**写入部分 JSON** 后返回 `false`。此时函数仅置 `body_len = 0` 并返回错误,但 `response->body` 中残留不完整 JSON。若上层调用者忽略返回值直接发送,客户端会收到截断/损坏的 JSON。
|
||
|
||
**修复**:失败前显式将 `response->body[0] = '\0'`。
|
||
|
||
---
|
||
|
||
### H14. 旋转期间状态锁释放导致脏读及角度不一致
|
||
- **文件**:`components/domain/src/direction/domain_direction_service.c`
|
||
- **函数**:`domain_rotate_to_angle_with_timeout`
|
||
- **行号**:302-364
|
||
|
||
```c
|
||
domain_direction_unlock_state(); // 释放 state mutex
|
||
// ... 电机运动期间 ...
|
||
esp_err_t motor_err = domain_runtime_stepper_rotate_angle_with_accel_timeout(...);
|
||
```
|
||
|
||
**问题**:释放 `s_state_mutex` 后、电机运动完成前,其他任务可获取 `s_state_mutex` 并读取 `s_current_angle`(此时仍是旧值),产生不一致快照。此外,若电机实际运动与指令存在偏差,`s_current_angle` 在运动后仅按理论值更新。
|
||
|
||
**修复**:旋转完成后重新获取锁更新角度,或记录旋转目标值供读取者使用。
|
||
|
||
---
|
||
|
||
### H15. HTTP 请求路径/方法 NULL/长度未校验
|
||
- **文件**:`components/domain/src/fortune/domain_user_config_server.c`
|
||
- **函数**:`user_config_http_handler`
|
||
- **行号**:338-364
|
||
|
||
```c
|
||
if (strcmp(request->path, "/") == 0) { // request->path 可能为 NULL
|
||
```
|
||
|
||
**问题**:对 `request->path` 直接使用 `strcmp`,未校验 `NULL` 或确保 NUL 终止;`request->method` 也未校验是否为合法枚举值。若平台层传递畸形请求,将导致空指针解引用或越界读取。
|
||
|
||
**修复**:增加 `request->path != NULL` 和 `request->method` 范围校验。
|
||
|
||
---
|
||
|
||
### H16. 启动顺序错误 — 平台总初始化晚于音频/LCD
|
||
- **文件**:`components/app_composition/src/runtime/app_composition_default_runtime.c`
|
||
- **函数**:`app_composition_run_plan_a_boot_sequence`
|
||
- **行号**:67-194
|
||
|
||
**问题**:启动流程中 `control_plane_play_boot_sound()` 和 `control_plane_init_lcd_device()` 在 `control_plane_init()`(内部调用 `domain_runtime_platform_init()` → `platform_init()`)之前执行。若音频或 LCD 的 platform 实现依赖 `platform_init()` 预先配置的 GPIO/DMA/时钟等资源,前 3 步将在平台未就绪时操作硬件。
|
||
|
||
**修复**:将 `platform_init()` 或等价的底层初始化移到 boot sequence 的最前面。
|
||
|
||
---
|
||
|
||
## 三、中(Medium)— 设计缺陷、资源泄漏或边界条件
|
||
|
||
### M1. `local_responding_event_worker` 队列永久泄漏
|
||
- **文件**:`components/control_plane/src/multimodal/control_plane_multimodal_session_service.c`
|
||
- **函数**:`stop_local_responding_event_worker()`
|
||
- **行号**:612-654
|
||
|
||
```c
|
||
if (s_local_responding_event_queue != NULL) {
|
||
xQueueReset(s_local_responding_event_queue); // 仅 reset,未删除
|
||
}
|
||
```
|
||
|
||
**问题**:队列在 stop 时仅被 `xQueueReset`,从未 `vQueueDelete`。反复启停 multimodal client 将导致队列句柄永久累积。
|
||
|
||
**修复**:`vQueueDelete(s_local_responding_event_queue)` 并将指针置 NULL。
|
||
|
||
---
|
||
|
||
### M2. `send_start` 发送成功后状态被 mark_transport_unavailable 干扰
|
||
- **文件**:`components/control_plane/src/multimodal/control_plane_multimodal_session_service.c`
|
||
- **函数**:`control_plane_multimodal_send_start()`
|
||
- **行号**:1843-1866
|
||
|
||
```c
|
||
err = domain_runtime_multimodal_transport_send_text("start", json);
|
||
// ...
|
||
if (!s_start_in_progress || strcmp(s_pending_start_task_id, task_id) != 0) {
|
||
return ESP_ERR_INVALID_STATE; // Start 已发到服务端,但本地拒绝记录
|
||
}
|
||
```
|
||
|
||
**问题**:`send_text` 成功返回后、再次加锁前,若 transport callback 线程调用 `mark_transport_unavailable()` 清空了 `s_start_in_progress`,则函数返回 `ESP_ERR_INVALID_STATE`。此时服务端已创建 task,客户端却无记录,形成 orphaned server task。
|
||
|
||
**修复**:在发送前加锁获取 task_id,发送完成后在锁内更新状态。
|
||
|
||
---
|
||
|
||
### M3. `process_text_json` 中 `is_finished` 跨事件污染
|
||
- **文件**:`components/control_plane/src/multimodal/control_plane_multimodal_session_service.c`
|
||
- **函数**:`process_text_json()`
|
||
- **行号**:1218, 1412-1423
|
||
|
||
```c
|
||
bool is_finished = cJSON_IsBool(finished) ? cJSON_IsTrue(finished) : false;
|
||
// ...
|
||
if (text_len != s_last_asr_text_len || is_finished) { ... }
|
||
```
|
||
|
||
**问题**:`is_finished` 在函数入口处基于 `output.finished` 计算一次。`SpeechContent` 事件(ASR 中间结果)通常不带 `finished` 字段,此时 `is_finished` 保留上一个事件(可能是 TTS `RespondingContent`)的值。若上一个事件 `finished=true`,当前 ASR 文本会被错误标记为 final。
|
||
|
||
**修复**:每个事件分支独立计算 `is_finished`,或使用事件类型判断而非共享变量。
|
||
|
||
---
|
||
|
||
### M4. 生日档案日期校验宽松 — 接受 2 月 31 日等非法日期
|
||
- **文件**:`components/domain/src/fortune/domain_fortune_store.c`
|
||
- **函数**:`domain_birth_profile_is_valid`
|
||
- **行号**:36-46
|
||
|
||
```c
|
||
profile->day >= 1 && profile->day <= 31 // 2月也可为31
|
||
```
|
||
|
||
**问题**:存储层接受所有月份的 day<=31。若数据绕过 Web 校验直接写入 NVS,非法日期将通过校验。
|
||
|
||
**修复**:增加按月份的日期上限校验,或统一使用 `is_valid_date` 函数。
|
||
|
||
---
|
||
|
||
### M5. HTTP 请求体无上限 — DoS/内存耗尽风险
|
||
- **文件**:`components/domain/src/fortune/domain_user_config_server.c`
|
||
- **函数**:`parse_profile`
|
||
- **行号**:198-212
|
||
|
||
```c
|
||
cJSON *root = cJSON_ParseWithLength((const char *)body, body_len);
|
||
```
|
||
|
||
**问题**:`body_len` 未设上限直接传入 `cJSON_ParseWithLength`。若攻击者发送数 MB 的畸形 JSON,将耗尽堆内存。
|
||
|
||
**修复**:在 platform HTTP server 层或 domain 层增加 `body_len <= MAX_JSON_BODY_SIZE` 校验。
|
||
|
||
---
|
||
|
||
### M6. 启动失败时资源未清理
|
||
- **文件**:`components/app_composition/src/runtime/app_composition_default_runtime.c`
|
||
- **函数**:`app_composition_run_plan_a_boot_sequence`
|
||
- **行号**:128-131, 164-167, 183-187
|
||
|
||
**问题**:若 `control_plane_init()`、`control_plane_prepare_runtime_storage()` 或 `control_plane_start_runtime_workers()` 失败,函数直接返回错误。但此前已初始化的音频子系统、LCD 设备、LCD 渲染任务等**没有任何 stop/deinit** 路径。
|
||
|
||
**修复**:在 boot sequence 中增加失败时的逆向清理逻辑(reverse cleanup)。
|
||
|
||
---
|
||
|
||
### M7. 旧版认证迁移写失败导致反复重试迁移
|
||
- **文件**:`components/domain/src/runtime/domain_runtime_adapters.c`
|
||
- **函数**:`domain_runtime_auth_store_load_record`
|
||
- **行号**:1005-1018
|
||
|
||
```c
|
||
esp_err_t save_err = platform_storage_set_blob(ns, key, out, actual);
|
||
if (save_err != ESP_OK) {
|
||
ESP_LOGW(TAG, "Persist migrated auth record failed");
|
||
} else {
|
||
ESP_LOGI(TAG, "Migrated legacy auth record");
|
||
}
|
||
return ESP_OK; // 无论 save_err 如何都返回 OK
|
||
```
|
||
|
||
**问题**:`save_err != ESP_OK` 时仅打印警告,仍返回 `ESP_OK`。下次加载同一条记录时,将再次执行迁移流程,造成反复读写 NVS。
|
||
|
||
**修复**:迁移写失败时应返回错误,或设置标志避免重复迁移。
|
||
|
||
---
|
||
|
||
### M8. 控制层直接调用 ESP-IDF 堆 API — 架构层旁路
|
||
- **文件**:`components/control_plane/src/runtime/control_plane_fortune_service.c`
|
||
- **函数**:`fortune_psram_first_calloc`
|
||
- **行号**:46-60
|
||
|
||
```c
|
||
void *ptr = heap_caps_calloc(count, size, MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT);
|
||
```
|
||
|
||
**问题**:`control_plane` 直接调用 `heap_caps_calloc` 和包含 `soc/soc_caps.h`。根据 `AGENTS.md` 的四层依赖约束,control_plane 不应直接调用 ESP-IDF 内部 API,而应通过 `platform` facade。
|
||
|
||
**修复**:将 PSRAM 分配封装为 `platform_psram_first_calloc()`,由 platform 层暴露给上层。
|
||
|
||
---
|
||
|
||
### M9. `wifi_portal_connect_sta` 持锁忙等 10 秒
|
||
- **文件**:`components/platform/src/wifi_portal_bridge.cpp`
|
||
- **函数**:`wifi_portal_connect_sta`
|
||
- **行号**:280-291
|
||
|
||
```c
|
||
while (esp_timer_get_time() < deadline_us) {
|
||
if (manager.IsConnected()) { return ESP_OK; }
|
||
vTaskDelay(pdMS_TO_TICKS(100));
|
||
}
|
||
```
|
||
|
||
**问题**:在持有 `s_portal_mutex` 递归锁的情况下进行最多 10 秒的轮询。期间所有需要该锁的操作(保存凭证、清除凭证)全部被阻塞。
|
||
|
||
**修复**:释放锁后轮询,或使用事件组/信号量等待连接完成。
|
||
|
||
---
|
||
|
||
### M10. 旋转互斥锁为非递归锁
|
||
- **文件**:`components/domain/src/direction/domain_direction_service.c`
|
||
- **函数**:`ensure_rotation_mutex`
|
||
- **行号**:99-125
|
||
|
||
```c
|
||
SemaphoreHandle_t created_mutex = xSemaphoreCreateMutex(); // 非递归
|
||
```
|
||
|
||
**问题**:`s_rotation_mutex` 是普通互斥锁(非递归)。若同一任务因回调或嵌套调用再次进入 `domain_rotate_to_angle_with_timeout`,将在 `domain_direction_lock_rotation` 死锁。
|
||
|
||
**修复**:改为 `xSemaphoreCreateRecursiveMutex()`。
|
||
|
||
---
|
||
|
||
### M11. `platform_storage_get_blob` / `get_string` 栈上 1.6KB 结构体
|
||
- **文件**:`components/domain/src/runtime/domain_runtime_adapters.c`
|
||
- **行号**:778, 870
|
||
|
||
```c
|
||
typedef struct { char jwt[1536]; ... } domain_auth_state_legacy_blob_t;
|
||
domain_auth_state_legacy_blob_t state = {0}; // 约1.6KB 栈分配
|
||
```
|
||
|
||
**问题**:该结构体分配在栈上,在栈空间受限的任务中可能导致栈溢出。
|
||
|
||
**修复**:改为 PSRAM 动态分配。
|
||
|
||
---
|
||
|
||
### M12. `control_plane_set_ptt_active()` 临界区内逻辑偏多
|
||
- **文件**:`components/control_plane/src/runtime/control_plane_runtime_workers.c`
|
||
- **函数**:`control_plane_set_ptt_active()`
|
||
- **行号**:293-335
|
||
|
||
**问题**:`portENTER_CRITICAL(&s_runtime_lock)` 内包含大量字段赋值与条件分支。在高频按键场景下会延长另一核心获取自旋锁的等待时间,影响中断延迟。
|
||
|
||
**修复**:将非必要字段移到临界区外,或使用轻量级原子操作替代。
|
||
|
||
---
|
||
|
||
## 四、低(Low)— 代码质量、可维护性和优化建议
|
||
|
||
### L1. `s_server_success_prompt_played` 永不清零
|
||
- **文件**:`components/control_plane/src/multimodal/control_plane_multimodal_runtime.c`
|
||
- **行号**:58
|
||
- **问题**:断连重连后成功提示音不再播放。确认是否为有意设计。
|
||
|
||
### L2. 多处 `cJSON_free` 与 `free` / `heap_caps_free` 混用
|
||
- **文件**:`control_plane_multimodal_device_auth.c`、`control_plane_multimodal_auth_service.c`
|
||
- **问题**:`cJSON_PrintUnformatted` 的结果使用 `cJSON_free()` 释放,而 PSRAM 分配的内存有时使用 `free()`。ESP-IDF 默认下 `cJSON_free` 等同于 `free`,但一旦 cJSON 库配置为自定义分配器,将导致分配/释放不匹配。
|
||
|
||
### L3. `control_plane_multimodal_generate_uuid()` 使用非加密安全随机数
|
||
- **文件**:`components/control_plane/src/multimodal/control_plane_multimodal_protocol.c`
|
||
- **行号**:102-124
|
||
- **问题**:`esp_random()` 基于硬件 RNG,但在某些启动阶段熵可能不足。对于 task_id 等协议标识符虽无安全要求,但若未来用于安全敏感场景需替换。
|
||
|
||
### L4. `audio_debug_dump_impl.c` UDP socket 无错误检查
|
||
- **文件**:`components/platform/src/multimodal/audio_pipeline/audio_debug_dump_impl.c`
|
||
- **行号**:95-127
|
||
- **问题**:`sendto` 返回值被忽略。若网络异常导致持续失败,无法及时发现 socket 异常。
|
||
|
||
### L5. `domain_runtime_post_json` 的 auth header 应改用栈缓冲区
|
||
- **文件**:`components/domain/src/runtime/domain_runtime_adapters.c`
|
||
- **行号**:477-511
|
||
- **问题**:每次调用都 `malloc` + `snprintf` + `free` 一个 Authorization header 字符串。token 最大 256 字节 + "Bearer " 前缀,总计约 264 字节,完全可以在栈上分配。
|
||
|
||
### L6. `control_plane_delete_task(NULL, task_with_caps)` 语义不清晰
|
||
- **文件**:`components/control_plane/src/runtime/control_plane_runtime_workers.c`
|
||
- **行号**:631
|
||
- **问题**:`vTaskDelete(NULL)` 会删除当前任务,但代码意图极不清晰。应加注释说明。
|
||
|
||
### L7. `control_plane_events.h` 的 event union 只有 key 成员
|
||
- **文件**:`components/control_plane/internal/control_plane_events.h`
|
||
- **行号**:50-56
|
||
- **问题**:定义了 WIFI、MULTIMODAL、LCD 等多种事件族,但 union 中只有 `key` 成员。若不打算通过事件队列传递其他事件,建议简化并加注释。
|
||
|
||
### L8. `platform_storage.c` NVS handle 频繁 open/close
|
||
- **文件**:`components/platform/src/runtime/platform_storage.c`
|
||
- **问题**:每次读写操作都调用 `nvs_open` + `nvs_close`,有固定开销。对高频 namespace 考虑缓存 handle。
|
||
|
||
### L9. `transport_ws.c` `s_headers[384]` 缓冲区偏小
|
||
- **文件**:`components/platform/src/multimodal/transport_ws/transport_ws.c`
|
||
- **行号**:158-159
|
||
- **问题**:Bearer token 255 字符 + workspace ID + header 格式可能超过 384 字节。建议增大至 512 字节。
|
||
|
||
### L10. `main.c` 忽略启动返回值
|
||
- **文件**:`main/main.c`
|
||
- **行号**:9-11
|
||
- **问题**:`(void)app_composition_start_default_runtime();` 启动失败被静默忽略。
|
||
|
||
---
|
||
|
||
## 五、修复优先级建议
|
||
|
||
### P0 — 必须立即修复(崩溃/永久功能丧失风险)
|
||
|
||
| 编号 | 问题 | 文件 |
|
||
|------|------|------|
|
||
| C1 | Watchdog 自删除导致多模态永久失效 | multimodal_runtime.c |
|
||
| C2 | Key Input ISR UAF | control_plane_key_input.c |
|
||
| C3 | 音频下游队列 UAF | audio_pipeline_runtime_impl.c |
|
||
| C4 | 音频任务强制删除 UAF | audio_pipeline_runtime_impl.c |
|
||
| C5 | 校准缺旋转锁 | domain_calibration_service.c |
|
||
| C6 | 角度归一化死循环 | domain_angle_math.c |
|
||
| C7 | WS 发送锁删除竞态 | transport_ws_runtime_impl.c |
|
||
| C8 | Timer 回调竞态删除 | audio_pipeline_runtime_impl.c |
|
||
| C9 | `is_connected` 对已销毁 client 操作 | transport_ws.c |
|
||
| C10 | LAN HTTP Server UAF | platform_lan_http_server.c |
|
||
| H1 | `s_started` 与 transport 状态不一致 | session_service.c |
|
||
| H2 | Session mutex 创建非线程安全 | session_service.c |
|
||
|
||
### P1 — 应尽快修复(竞态/数据损坏/功能异常)
|
||
|
||
| 编号 | 问题 | 文件 |
|
||
|------|------|------|
|
||
| H3 | Device Auth 损坏数据继续初始化 | device_auth.c |
|
||
| H4 | PTT teardown TOCTOU | runtime_workers.c |
|
||
| H5 | PTT foreground_attempts 不一致 | runtime_workers.c |
|
||
| H6 | WS 重组缓冲区无互斥 | transport_ws_workers_impl.c |
|
||
| H7 | wait_worker_task_exit 无原子读取 | transport_ws_workers_impl.c |
|
||
| H8 | LCD 渲染任务强制删除锁永久占用 | lcd_direct.c |
|
||
| H9 | 步进电机旋转饿死其他任务 | platform_stepper_driver.c |
|
||
| H10-H11 | 多处强制删除任务 | factory_cli.c / system_events.c |
|
||
| H12 | 浮点精确比较失败 | domain_direction_catalog.c |
|
||
| H13 | JSON 打印脏数据 | domain_user_config_server.c |
|
||
| H16 | 启动顺序错误 | app_composition_default_runtime.c |
|
||
|
||
### P2 — 建议优化(资源泄漏、设计缺陷、边界条件)
|
||
|
||
| 编号 | 问题 | 文件 |
|
||
|------|------|------|
|
||
| M1 | local_responding_event 队列泄漏 | session_service.c |
|
||
| M2 | send_start orphaned task | session_service.c |
|
||
| M3 | is_finished 跨事件污染 | session_service.c |
|
||
| M4-M5 | 日期校验/HTTP body 上限 | fortune_store.c / user_config_server.c |
|
||
| M6 | 启动失败资源未清理 | app_composition_default_runtime.c |
|
||
| M7 | 认证迁移反复重试 | domain_runtime_adapters.c |
|
||
| M8 | 架构层旁路 | control_plane_fortune_service.c |
|
||
| M9 | Wi-Fi 连接持锁忙等 | wifi_portal_bridge.cpp |
|
||
| M10 | 旋转锁非递归 | domain_direction_service.c |
|
||
|
||
### P3 — 可后续处理(代码质量、性能、文档)
|
||
|
||
L1-L10 及其他低优先级项。
|
||
|
||
---
|
||
|
||
## 六、总结
|
||
|
||
本次深度审查在三个层面共发现 **10 个 Critical、16 个 High、12 个 Medium、10 个 Low** 级别问题。
|
||
|
||
**最危险的发现**:
|
||
1. **Watchdog 任务自删除**(C1)会导致设备空闲超时后多模态功能**永久丧失**,除非重启
|
||
2. **多处 Use-After-Free**(C2-C4, C9-C10)在特定时序下会直接导致系统崩溃
|
||
3. **校准缺旋转锁**(C5)会导致步进电机并发冲突,可能损坏机械结构
|
||
4. **角度归一化死循环**(C6)对畸形输入会挂死任务,触发看门狗复位
|
||
|
||
**建议立即处理 P0 列表中的 12 项**,它们覆盖了崩溃、永久功能丧失和硬件损坏风险。P1 中的竞态条件和数据损坏问题应在下一个迭代中跟进。
|