[components][sal] Fix SIOCGIFCONF buffer overflow - #11774
Conversation
|
👋 感谢您对 RT-Thread 的贡献!Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范,请在你的仓库中执行以下步骤运行代码格式化工作流(如果格式化CI运行失败)。 🛠 操作步骤 | Steps
完成后,提交将自动更新至 如有问题欢迎联系我们,再次感谢您的贡献!💐 |
📌 Code Review Assignment🏷️ Tag: componentsReviewers: @Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2026-09-03 11:00 CST)
📝 Review Instructions
|
|
4a4cfd9 to
35e92a9
Compare
There was a problem hiding this comment.
🟡 Changes recommended
NULL size queries remain broken, a pointer race exists, and the regression test is not committed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes SAL SIOCGIFCONF buffer overflow and stack-data exposure. 修复 SAL 缓冲区越界及栈数据泄漏。
Changes:
- Validates buffer length and bounds copying.
- Zero-initializes returned interface records.
- Preserves the caller’s buffer pointer.
File summaries
| File | Description |
|---|---|
components/net/sal/src/sal_socket.c |
Adds bounded, initialized interface-list copying. |
Review details
Suppressed comments (1)
components/net/sal/src/sal_socket.c:1597
- [Security/安全]: Copy through the pointer that was checked / 使用已检查的指针执行复制
English: The NULL check uses the cached ifc_buf, but the write rereads ifconf_tmp->ifc_ifcu.ifcu_buf. Because LWP sys_ioctl passes the user-owned argument through directly, another user thread can change that field between the check and copy, redirecting the kernel write or causing a NULL fault. Use the cached pointer for the destination so the check and write operate on the same value.
中文:空指针检查使用缓存的 ifc_buf,但写入时又重新读取 ifconf_tmp->ifc_ifcu.ifcu_buf。由于 LWP 的 sys_ioctl 会直接传递用户持有的参数,另一个用户线程可在检查与复制之间修改该字段,从而重定向内核写入或触发空指针故障。应使用缓存指针作为目标,确保检查和写入基于同一个值。
rt_memcpy(ifconf_tmp->ifc_ifcu.ifcu_buf + copied_size, &sal_ifreq_temp, sizeof(struct sal_ifreq));
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
35e92a9 to
6a93e7e
Compare
msh />utest_list
[I/utest] Commands list :
[I/utest] [testcase name]:components.net.sal.socket_basic; [run timeout]:30
msh />utest_r
utest_run
msh />utest_run c
components.net.sal.socket_basic
msh />utest_run components.net.sal.socket_basic
[I/utest] [==========] [ utest ] loop 1/1
[I/utest] [==========] [ utest ] started
[I/utest] [----------] [ testcase ] (components.net.sal.socket_basic) started
[I/utest] ===========================================
[I/utest] Starting SAL Socket Basic API Tests
[I/utest] ===========================================
[I/utest] Checking network device status...
[I/utest] Network test socket created successfully: 0
[I/utest] Using loopback IP address for testing: 127.0.0.1
[I/utest] Network device appears operational
[I/utest] [==========] utest unit name: (TC_sal_socket_create)
[I/utest] Starting TC_sal_socket_create tests...
[I/utest] Testing TCP socket creation...
[I/utest] TCP socket created: 0
[I/utest] Closed socket 0
[I/utest] Testing UDP socket creation...
[I/utest] UDP socket created: 0
[I/utest] Closed socket 0
[I/utest] Testing invalid family parameter...
[E/sal.skt] Invalid family: -1 (must be 0 ~ 47)
[E/sal.skt] SAL socket protocol family input failed, return error -1.
[I/utest] Invalid family result: -1
[I/utest] Testing invalid type parameter...
[E/sal.skt] Invalid type: -1 (must be 0 ~ 524289)
[E/sal.skt] SAL socket protocol family input failed, return error -2.
[I/utest] Invalid type result: -2
[I/utest] Testing invalid protocol parameter...
[E/sal.skt] Invalid protocol: -1 (must be 0 ~ 255)
[E/sal.skt] SAL socket protocol family input failed, return error -4.
[I/utest] Invalid protocol result: -4
[I/utest] TC_sal_socket_create tests completed
[I/utest] [==========] utest unit name: (TC_sal_socket_bind)
[I/utest] Starting TC_sal_socket_bind tests...
[I/utest] Creating socket for bind test on port 9000...
[I/utest] Created socket 0 (domain=2, type=1, protocol=0)
[I/utest] Attempting to bind socket 0 to port 9000...
[I/utest] Bind result: 0 (expected 0)
[I/utest] Closed socket 0
[I/utest] Skipping NULL address bind test (would cause assertion)
[I/utest] Testing bind with invalid socket...
[I/utest] Invalid socket bind result: -1 (expected -1)
[I/utest] TC_sal_socket_bind tests completed
[I/utest] [==========] utest unit name: (TC_sal_socket_listen)
[I/utest] Starting TC_sal_socket_listen tests...
[I/utest] Creating socket for listen test...
[I/utest] Created socket 0 (domain=2, type=1, protocol=0)
[I/utest] Binding socket 0 for listen test on port 9002...
[I/utest] Testing listen with backlog 5 on socket 0...
[I/utest] Listen result: 0 (expected 0)
[I/utest] Testing listen with invalid backlog (-1)...
[I/utest] Invalid backlog listen result: 0
[I/utest] Closed socket 0
[I/utest] Testing listen on invalid socket...
[I/utest] Invalid socket listen result: -1 (expected -1)
[I/utest] TC_sal_socket_listen tests completed
[I/utest] [==========] utest unit name: (TC_sal_socket_connect)
[I/utest] Starting TC_sal_socket_connect tests...
[I/utest] Setting up test server on port 9004...
[I/utest] Setting up threaded test server on port 9004
[I/utest] Created server thread
[I/utest] Created client thread
[I/utest] Started server and client threads
[I/utest] Starting client thread for port 9004
[I/utest] Starting server thread on port 9004
[I/utest] Created socket 0 (domain=2, type=1, protocol=0)
[I/utest] Server socket 0 bound to port 9004
[I/utest] Server socket 0 listening with backlog 5
[I/utest] Server ready and signaled on port 9004
[I/utest] Created socket 1 (domain=2, type=1, protocol=0)
[I/utest] Attempting to connect to server 127.0.0.1:9004
[I/utest] Connect result: -1
[E/utest] Connection to server failed: return=-1, errno=104
[I/utest] Closed socket 1
[I/utest] Cleaned up threads and event
[I/utest] Test server setup partially successful (connect verified)
[I/utest] Test server setup completed successfully
[I/utest] Testing connect to invalid address 192.168.999.999...
[I/utest] Created socket 1 (domain=2, type=1, protocol=0)
[I/utest] Invalid address connect result: -1 (expected -1)
[I/utest] Closed socket 1
[I/utest] TC_sal_socket_connect tests completed
[I/utest] [==========] utest unit name: (TC_sal_socket_accept)
[I/utest] Starting TC_sal_socket_accept tests...
[I/utest] Creating server socket...
[I/utest] Created socket 1 (domain=2, type=1, protocol=0)
[I/utest] Binding server socket 1 to port 9006...
[I/utest] Starting to listen on server socket 1...
[I/utest] Testing accept with timeout on socket 1...
[I/utest] Accept timed out as expected: -1
[I/utest] Closed socket 1
[I/utest] Testing accept on invalid socket...
[I/utest] Invalid socket accept result: -1 (expected -1)
[I/utest] TC_sal_socket_accept tests completed
[I/utest] [==========] utest unit name: (TC_sal_socket_send_recv)
[I/utest] Starting TC_sal_socket_send_recv tests...
[I/utest] Setting up test server on port 9008...
[I/utest] Setting up threaded test server on port 9008
[I/utest] Created server thread
[I/utest] Created client thread
[I/utest] Started server and client threads
[I/utest] Starting client thread for port 9008
[I/utest] Starting server thread on port 9008
[I/utest] Created socket 1 (domain=2, type=1, protocol=0)
[I/utest] Server socket 1 bound to port 9008
[I/utest] Server socket 1 listening with backlog 5
[I/utest] Server ready and signaled on port 9008
[I/utest] Created socket 2 (domain=2, type=1, protocol=0)
[I/utest] Attempting to connect to server 127.0.0.1:9008
[I/utest] Connect result: -1
[E/utest] Connection to server failed: return=-1, errno=104
[I/utest] Closed socket 2
[I/utest] Cleaned up threads and event
[I/utest] Test server setup partially successful (connect verified)
[I/utest] Testing send/recv with data exchange...
[I/utest] Created socket 2 (domain=2, type=1, protocol=0)
[I/utest] Created socket 3 (domain=2, type=1, protocol=0)
[I/utest] Server socket 2 bound to port 9009
[I/utest] Server socket 2 listening
[E/utest] Client connect failed: -1
[I/utest] Closed socket 41
[I/utest] Closed socket 3
[I/utest] Closed socket 2
[I/utest] Cleaned up test connection sockets
[I/utest] TC_sal_socket_send_recv tests completed
[I/utest] [==========] utest unit name: (TC_sal_socket_udp_communication)
[I/utest] Starting TC_sal_socket_udp_communication tests...
[I/utest] Created socket 2 (domain=2, type=2, protocol=0)
[I/utest] Created socket 3 (domain=2, type=2, protocol=0)
[I/utest] Server socket 2 bound to port 9010
[I/utest] Client socket 3 bound to port 9110
[I/utest] Sending 21 bytes from client to server...
[I/utest] Client sent 21 bytes to server
[W/utest] UDP recv failed, may be expected: -1 bytes received, errno=11
[I/utest] Closed socket 2
[I/utest] Closed socket 3
[I/utest] TC_sal_socket_udp_communication tests completed
[I/utest] [==========] utest unit name: (TC_sal_socket_getpeername_getsockname)
[I/utest] Starting TC_sal_socket_getpeername_getsockname tests...
[I/utest] Created socket 2 (domain=2, type=1, protocol=0)
[I/utest] Server socket 2 bound to port 9012
[I/utest] Testing getsockname on socket 2...
[I/utest] Getsockname result: 0 (expected 0)
[I/utest] Closed socket 2
[I/utest] Testing getsockname/getpeername on invalid socket...
[I/utest] Invalid socket getsockname result: -1 (expected -1)
[I/utest] Invalid socket getpeername result: -1 (expected -1)
[I/utest] TC_sal_socket_getpeername_getsockname tests completed
[I/utest] [==========] utest unit name: (TC_sal_socket_siocgifconf)
[I/utest] Created socket 2 (domain=2, type=2, protocol=0)
[E/sal.skt] ifconfig: network interface device list buffer size error.
[I/utest] Closed socket 2
[I/utest] [==========] utest unit name: (TC_sal_socket_close)
[I/utest] Starting TC_sal_socket_close tests...
[I/utest] Testing close valid socket...
[I/utest] Created socket 2 (domain=2, type=1, protocol=0)
[I/utest] Closing socket 2...
[I/utest] Testing close invalid socket...
[I/utest] Testing double close...
[I/utest] Created socket 2 (domain=2, type=1, protocol=0)
[I/utest] Double closing socket 2 (should be safe)
[I/utest] TC_sal_socket_close tests completed
[I/utest] ===========================================
[I/utest] SAL Socket Basic API Tests Completed
[I/utest] ===========================================
[I/utest] [ PASSED ] [ result ] testcase (components.net.sal.socket_basic)
[I/utest] [----------] [ testcase ] (components.net.sal.socket_basic) finished
[I/utest] [==========] [ utest ] 1 tests from 1 testcase ran.
[I/utest] [ PASSED ] [ result ] 1 tests.
[I/utest] [==========] [ utest ] finished |
拉取/合并请求描述:(PR description)
[
Fixes #11763
为什么提交这份PR (why to submit this PR)
修复 SAL 模块,用户获取网卡信息可能写越界 / 栈帧内存泄漏的问题
你的解决方案是什么 (what is your solution)
初始化 sal_ifreq_temp 结构体,并对 buffer_size 进行长度校验
请提供验证的bsp和config (provide the config and bsp)
rt-thread/bsp/qemu-vexpress-a9
UT 代码如下
验证日志
https://github.com/Huoyanlifusu/rt-thread/actions/runs/33349075279/job/99358741679
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality:
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up