Skip to content

fix: fs_watch calls trace_file_init instead of mountsnoop_init with path argument - #112

Open
JoeSergen wants to merge 1 commit into
DKapture:mainfrom
JoeSergen:fix/fs-watch-wrong-init
Open

JoeSergen wants to merge 1 commit into
DKapture:mainfrom
JoeSergen:fix/fs-watch-wrong-init

Conversation

@JoeSergen

@JoeSergen JoeSergen commented Jul 25, 2026

Copy link
Copy Markdown

Summary

Fix copy-paste bug in so/dkapture.cpp where fs_watch() called trace_file_init() instead of mountsnoop_init() in its path-specific else branch.

Root Cause

int dkapture::fs_watch(const char *path, DKCallback cb, void *ctx)
{
    if (!cb)    return mountsnoop_deinit();        // ✓ correct
    if (!path)  return mountsnoop_init(1, ...);     // ✓ correct
    else        return trace_file_init(3, ...);     // ✗ BUG: should be mountsnoop_init
}

This is a copy-paste error from file_watch(). The developer correctly changed the deinit and null-path branches to mountsnoop_*, but the else branch was missed.

Why mountsnoop_init is Correct

fs_watch monitors filesystem-level mount/umount events, not per-file I/O. Evidence from the BPF kernel side:

mountsnoop_init — Filesystem Event Monitoring

User-space (observe/mountsnoop.cpp): The -p argument is stored as a path prefix and written to a BPF char[4096] array map.

Kernel-space (bpf/observe/mountsnoop.bpf.c:44-60): filter_path() performs path prefix matching against mount source/target paths:

static bool filter_path(const char *path, int n) {
    char *rule_path = bpf_map_lookup_elem(&filter, &key);
    if (!rule_path || rule_path[0] == '\0') return true;
    if (!path) return false;
    return strncmp(path, rule_path, n) == 0;
}

Monitored events: sys_enter_mount, sys_enter_umount, fsopen, fsmount, move_mount, etc. — all filesystem-level syscalls.

trace_file_init — Per-File I/O Monitoring

User-space (observe/trace-file.cpp:1206-1212): The -p argument is used to open() + fstat() the file, resolving it to an inode:

target_fd = open(rule.path, O_RDONLY);
fstat(target_fd, &statbuf);
rule.inode = statbuf.st_ino;

Kernel-space (bpf/observe/trace-file.bpf.c:128-146): find_file_inode matches by inode+dev:

if (rule->not_inode)  strncmp(path, rule->path, PATH_MAX);  // path matching
else {
    rule->dev != file->f_path.mnt->mnt_sb->s_dev;           // device matching
    rule->inode != BPF_CORE_READ(file, f_inode, i_ino);     // inode matching
}

Monitored events: vfs_open, vfs_read, vfs_write, vfs_chmod, vfs_chown, vfs_removexattr — all per-file I/O operations.

Semantic Mismatch

trace_file_init mountsnoop_init
Monitors Single file (by inode) Filesystem mount/umount events
-p semantics File path → inode lookup Path prefix for filtering mount source/target
BPF hooks fexit/vfs_* file ops tracepoint/.../sys_enter_mount etc.

Calling trace_file_init from fs_watch would open the given path as a regular file, resolve its inode, and attach VFS-level file I/O hooks — completely unrelated to filesystem event monitoring.

Changes

  • so/dkapture.cpp:388: Changed trace_file_init to mountsnoop_init (1 line)

Verification

  • Both functions have identical signatures: int(int argc, char** argv, DKCallback cb, void* ctx)
  • No ABI changes, no header changes, no build dependency changes

Closes #111

…ath argument

When fs_watch() is called with a non-null path, the else branch
incorrectly calls trace_file_init() instead of mountsnoop_init().
This is a copy-paste error from file_watch(). The correct behavior
is to initialize mountsnoop for filesystem event monitoring.

Signed-off-by: JoeSergen <jxq142857@163.com>
@dkapture-ci-bot

Copy link
Copy Markdown
Contributor

你好,这是对 libdkapture#112 fix: fs_watch calls trace_file_init instead of mountsnoop_in 的评审。

本轮为 DKapture 两仓(libdkapture / dkapture-bpf)全部以 fix 开头 open PR 的批量评审,共 17 个;汇总表如下,本 PR 加粗。逐项详评见分隔线下方。

PR 标题 作者 规模 结论 主要风险
#149 fix(net-traffic): initialize rules before loadin yuKing123-king +19/-9 可合并 规则安装顺序与其它工具约定不一致;本仓改动无正确性缺陷
#147 fix(net-filter): abort rule loading on invalid c yuKing123-king +9/-3 需修改 失败时 clear_rules() 波及 add_rule() 装入的规则;空白注释行导致整个加载失败
#141 fix(dkapture): honor parsed pid in read(vector<p JoeSergen +23/-2 需修改 unsafe_find 单记录语义使 /proc//fd 只回调一次;返回值语义漂移
#140 fix(dkapture): return bytes read, not remaining JoeSergen +10/-4 可合并 无阻塞风险;与文档契约对齐,仓内无调用方依赖旧语义
#114 fix: replace manual destructor calls in construc JoeSergen +28/-7 需修改 捆绑了与 open PR #112 逐字节相同的 fs_watch 修复,跨 PR 重复
#125 fix(lsof): start ringbuf consumer before iterato yuKing123-king +17/-3 需修改 线程启动后错误路径仍 goto err_out → 释放运行中 rb(UAF)+ 线程泄漏
#112 fix: fs_watch calls trace_file_init instead of m JoeSergen +1/-1 可合并 无;仅修一处复制粘贴错误
#103 fix: make power-snoop internal symbols static fo yuKing123-king +3/-2 可合并 改动无害;但 PR 描述的 multiple definition 在当前构建配置下无法复现
#119 fix(syscall-stat): stop skipping syscall key 0 d yuKing123-king +18/-6 可合并 循环终止四条路径已逐一核实;缩进与提交拆分小问题
#115 fix(syscall-stat): improve builtin flow yuKing123-king +204/-41 需修改 三处 bpf_get_map_fd 错误路径未设 ret,最终 return ret 误报成功
#117 fix(trace-signal): validate invalid command line yuKing123-king +190/-29 需修改 BUILTIN 测试入口未接入 test/Makefile,不可达;register_signal 残留
#102 fix(trace-exec): reject invalid command line arg yuKing123-king +135/-22 需修改 BUILTIN 入口同样未接入构建;-h 退出码 0→1 属未说明的行为变更
#97 fix(so): reuse pinned dkapture bpf objects corre yuKing123-king +1/-1 需修改 test mock 仍按 map- 前缀命名 pin,合并后 gtest FindMap 用例失败
#35 fix(pagefault): add max_entries and value_size f yuKing123-king +7/-3 可合并 修复真实,但已被 main 上等效修复 1e90486 取代,建议确认后关闭
#32 fix(peek-fd): correct args field name from mvlen yuKing123-king +1/-1 可合并 无功能风险;标题/描述与实际改动方向不符
#30 fix(trace-signal): avoid inflight event key coll yuKing123-king +36/-22 需修改 sys_exit_kill 的 !rule 早退路径仍泄漏 inflight 条目
#29 fix(syscall-stat): replace exec fexit with kprob yuKing123-king +93/-13 阻塞 exec 路径从 struct filename* 本身读字符串,-f 过滤将完全失效

本 PR 评审详情

作者: JoeSergen | 规模: +1/-1 | 文件: 1
结论: 可合并
主要风险: 无;仅修一处复制粘贴错误。

总体结论: fs_watch(path) 带 path 的 else 分支原先调用 trace_file_init(3,...),与同函数另外两个分支(cb 为空调用 mountsnoop_deinit、path 为空调用 mountsnoop_init(1,...),so/dkapture.cpp:374,380 仓库行号)不一致。改为 mountsnoop_init(3, {"dkapture","-p",path},...) 正确:mountsnoop_init 在 BUILTIN 模式下存在(observe/mountsnoop.cpp:471),签名与 dkapture.cpp:364-369 的前置声明一致(DKapture::DKCallback);且 mountsnoop 的 -p 选项实际是路径过滤而非 PID 过滤(observe/mountsnoop.cpp:81-88 解析进 filter_path,:533 写入 "filter" map;BPF 侧 repos/dkapture-bpf/observe/mountsnoop.bpf.c:44-56 用其匹配 mount 的 source/target),传入文件系统路径语义自洽。

主要问题: 无
次要建议:

  • test/mountsnoop-test.cpp:22 — 现有测试直接调用 mountsnoop_init,未覆盖 fs_watch(path) 这条 API 路径,而本 bug 恰发生在该封装层;建议补一个 fs_watch 带 path 的用例防回归。
  • observe/mountsnoop.cpp:67 — -p 的 argp 帮助文本写 "Process ID to trace",实现却按路径过滤,易误导后续开发者再次犯同类复制粘贴错误(先于本 PR 存在,可另行修复)。

亮点:

  • commit message 带完整根因分析正文(Summary/Root Cause),符合仓库规范。

commit message: 符合规范(fix: 标题 + 根因正文 + Signed-off-by)
已有讨论: 无

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: fs_watch calls trace_file_init instead of mountsnoop_init with path argument

2 participants