Repository navigation
[dfs][9pfs] fix: clamp server-controlled lengths to the received reply - #11918
usualtreee wants to merge 1 commit into
Conversation
The 9PFS client trusted server-controlled length fields: - dfs_9pfs_read() copied READ3resok.count bytes into the caller's buffer without comparing count with the requested length or the decoded data.data_len, and advanced the buffer by the request length instead of the copied byte count. - dfs_9pfs_getdents() discarded the number of bytes actually received and then trusted the in-packet count plus every per-record stat_size and name length, so a crafted reply made the parser walk past the receive buffer (ASan use-after-poison, read 8.3 MB into an 8 MB heap region). Pass the actually-received byte count to p9_transaction() and clamp every wire-supplied length against it; reject replies shorter than the count field. In getdents, stop when the remaining bytes cannot hold the next record, when a record overruns the received data, or when a name overruns its record. Verified on bsp/simulator (x86-64, ASan) with a fake 9P server replying to Tread with an 11-byte packet whose count field claims ~2 GB: before the fix the reader aborts inside get_rx_value16_of()/dfs_9pfs_getdents(); after the fix the reply is rejected and the process survives (no ASan report, empty listing), while a well-formed listing still parses correctly. Refs: RT-Thread#11289
|
👋 感谢您对 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-10-22 19:36 CST)
📝 Review Instructions
|
|
我在尝试独立复现并验证这个 PR，环境已经备好，但缺一份东西，希望能补一下。 环境已就绪
一个配置上的坑，建议补进 PR 描述PR 里给的配置清单是 CONFIG_RT_USING_DFS_V1 / CONFIG_RT_USING_DFS_9PFS / CONFIG_RT_USING_ADT / CONFIG_RT_USING_ADT_BITMAP。照着做的人会发现在 �sp/simulator 的 menuconfig 里根本搜不到 RT_USING_DFS_9PFS，原因是它的 Kconfig 有隐藏依赖：
必须先打开 RT_USING_MEMHEAP，这个选项才会出现。我在这一步卡了一会儿，建议在 PR 描述的配置清单里补一条 CONFIG_RT_USING_MEMHEAP=y。 缺的是测试用例PR 只改了 components/dfs/dfs_v1/filesystems/dfs_9pfs.c 一个文件；#11289 那条评论里有完整补丁、有 ASAN 报告、有根因分析，但没有测试代码。评论里提到的 �vil9p_test、�san-report-run9-clean.txt、 ixed_evil.log 在这些地方都找不到。 能不能把那个假 9P 服务器的用例源码提供一下（贴评论、gist、或直接放进 �sp/simulator/ 下的测试目录都行）？理由不只是为了我复现：
另一条环境限制，也许影响 CI 方案Windows 上的 MinGW 不带 libasan：
也就是说这个用例在 Windows 贡献者机器上跑不了，只能 Linux/WSL。如果打算纳入 CI，跑在 Linux runner 上是前提。 拿到用例后我就做 before/after 双跑（master 与你的分支各一次），把两侧完整日志贴上来。如果维护者更希望先看代码评审、用例以后再补，也请说明。 |
|
补一条刚才编译时发现的问题：照 PR 里给的 BSP + 配置，9PFS 在当前 master 上编译不过。 复现�sp/simulator，按上一条评论的方式补齐 RT_USING_MEMHEAP + RT_USING_DFS_9PFS + RT_USING_ADT + RT_USING_ADT_BITMAP，scons -j8：
原因
而 dfs_9pfs.c:20 又 include 了 <drivers/misc.h>。 但 sim 架构没有这个头文件。libcpu/sim/ 下的全部内容：
全树共 22 个 cpuport.h（libcpu/aarch64/common/include/、libcpu/arm/*、部分 BSP 里），没有任何一个属于 sim 架构。 所以 Linux 侧的 simulator 构建里，凡是 include <drivers/misc.h> 的源文件都会失败——9PFS 是其中之一。 想确认的事
这条不影响我对补丁本身的判断（补丁逻辑我看过，收敛思路是对的），但它挡住了独立复现这条路。等这两点清楚了，加上上一条提到的用例，我就能把 before/after 双跑做出来。 |
感谢准备环境，两个问题都成立，逐条回复： 1. 配置缺 MEMHEAP — 你说得对。 2. 测试用例 — 见附件 3. 关于进 CI — 同意你的判断。 |
确认，这是我本地的一份临时绕行，不是 PR 的一部分： 解决方式：在本地加了 你的判断是对的： 关于范围：同意你的看法，给 sim 补一个 |
拉取/合并请求描述：(PR description)
为什么提交这份PR (why to submit this PR)
修复 issue #11289：9PFS 客户端在
dfs_9pfs_read()/dfs_9pfs_getdents()中信任服务器可控的长度字段，造成越界读（ASAN use-after-poison，读取越过接收缓冲区 8.3 MB）和目录信息泄露。p9_transaction()的返回字节数被丢弃（RT_NULL），随后代码直接使用报文里的count、每条记录的stat_size和name_len，没有任何与实际收到数据量的比对；当服务器返回一个count巨大但实际很短的报文时，解析器会一路读越接收缓冲区。正常情况下，被跳过的.记录不会走到fd->pos >= end退出条件，解析器可持续前进。你的解决方案是什么 (what is your solution)
dfs_9pfs_read()/dfs_9pfs_getdents()均把实际收到的字节数传给p9_transaction()（&rx_size），并校验rx_size >= P9_MSG_READ_DATA（否则返回P9_READ_UNEXPECTED_DATA/-EIO）；count收敛到rx_size - P9_MSG_READ_DATA（不信任报文值）；sizeof(rt_uint16_t)时停止；stat_size不得超出剩余数据；记录至少能容纳到 name 长度字段；name_len不得越过当前记录；got（got <= set并不充分，因为set本身可能超过接收容量）。修改文件：
components/dfs/dfs_v1/filesystems/9pfs/dfs_9pfs.c（+58/-4）请提供验证的bsp和config (provide the config and bsp)
-fsanitize=address）CONFIG_RT_USING_MEMHEAP=y（必需，RT_USING_DFS_9PFS的 Kconfig 依赖它）、CONFIG_RT_USING_DFS_V1=y、CONFIG_RT_USING_DFS_9PFS=y、CONFIG_RT_USING_ADT=y、CONFIG_RT_USING_ADT_BITMAP=y（模拟器 BSP 的 9PFS 验证配置）验证方法：注册一个假的内存 9P 服务器（transport 回调），正常完成握手（Tversion/Tattach/Tstat/Topen/Tclunk），但对 Tread 返回 11 字节报文且
count字段谎报约 2 GB 目录数据；接收缓冲区后方铺满伪造的.记录并用 ASAN poison 做守卫带。evil9p_test触发AddressSanitizer: use-after-poison，读取位置位于 8 MB 堆区之内 8334515 字节处（get_rx_value16_of←dfs_9pfs_getdents），进程 ABORT；listing done (survived), errno=0，无 ASAN 报告，进程存活）；正常目录解析不受影响（entry: hello.txt、entry: subdir）。证据日志（附在 issue 评论中）：
asan-report-run9-clean.txt（修复前）、fixed_evil.log/fixed_normal.log（修复后）。补丁在最新 master 上git apply --check通过（dfs_9pfs.c自补丁基点未变化）。当前拉取/合并请求的状态 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