fix: Fix search fail after mounting ulnfs

[wangrong1069]

Summary by Sourcery

Resolve path resolution issues by using shared mount information when computing event and blacklist paths.

Bug Fixes:

• Fix incorrect event and blacklist path resolution after new filesystems are mounted by deriving full paths from the current MountInfo state.

Enhancements:

• Initialize and reuse MountInfo earlier in the default event handler lifecycle so it is available during initial indexing and blacklist path processing.

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wangrong1069

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[github-actions]

TAG Bot

TAG: 7.0.38
EXISTED: no
DISTRIBUTION: unstable

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Propagate MountInfo into path resolution utilities and event handling so that event/indexing/blacklist paths are resolved using the current mount table (including ulnfs) instead of querying mountpoints ad‑hoc, fixing search failures after mounting ulnfs.

Sequence diagram for resolving event paths with MountInfo-aware get_full_path

sequenceDiagram
  participant Handler as default_event_handler
  participant Mount as MountInfo
  participant Helpers as path_helpers
  participant Tools as tools
  participant FS as filesystem_api

  Handler->>Mount: mount_info_new()
  activate Mount
  Mount-->>Handler: mount_info_
  deactivate Mount

  loop for each origin_path in config_.indexing_paths
    Handler->>Helpers: get_event_path(mount_info_, origin_path, indexing_items_)
    activate Helpers
    Helpers->>FS: exists(origin_path, ec)
    FS-->>Helpers: bool
    alt path exists
      Helpers->>Tools: get_full_path(mount_info_, origin_path.c_str())
      activate Tools
      Tools->>FS: lstat(path, &st)
      FS-->>Tools: int
      alt lstat ok
        Tools->>Mount: mount_info_get_device_mount_point(mount_info_, st.st_dev)
        Mount-->>Tools: mountpoint
        Tools-->>Helpers: event_path
      else lstat failed
        Tools-->>Helpers: NULL
      end
      deactivate Tools
      alt event_path != NULL
        Helpers-->>Handler: event_path_with_slash
      else event_path == NULL
        Helpers-->>Handler: origin_path_with_slash
      end
    else path does not exist
      Helpers-->>Handler: "" (empty string)
    end
    deactivate Helpers
  end

Loading

Class diagram for updated event handling and path resolution with MountInfo

classDiagram

class default_event_handler {
  -event_handler_config config_
  -GAsyncQueue* event_queue_
  -GThread* event_filter_thread_
  -GAsyncQueue* config_event_queue_
  -MountInfo* mount_info_
  -std_vector_indexing_item indexing_items_
  -std_vector_string event_path_blocked_list_
  +default_event_handler(event_handler_config config)
  +void handle_config_event()
}

class MountInfo {
  +mount_info_new() MountInfo*
  +mount_info_dump(MountInfo* mount_info) gchar*
  +mount_info_get_device_mount_point(MountInfo* mount_info, dev_t device) const gchar*
}

class tools {
  +char* get_full_path(MountInfo* mount_info, const char* path)
}

class path_helpers {
  +std_string determine_blacklist_path(MountInfo* mount_info, const char* path)
  +std_string get_event_path(MountInfo* mount_info, const std_string& origin_path, const std_vector_indexing_item& indexing_items)
}

class filesystem_api {
  +bool exists(const std_string& path, std_error_code& ec)
  +int lstat(const char* path, struct_stat* st)
}

class indexing_item {
}

%% Relationships
default_event_handler *-- MountInfo : owns
path_helpers ..> tools : uses
path_helpers ..> filesystem_api : uses
path_helpers ..> indexing_item : uses
tools ..> MountInfo : queries mount points
filesystem_api <.. tools : calls lstat
filesystem_api <.. path_helpers : calls exists

default_event_handler ..> path_helpers : uses for indexing_paths and blacklist_paths

Loading

File-Level Changes

Change	Details	Files
Pass MountInfo into path resolution helpers and event handler logic so event and blacklist paths resolve correctly on newly mounted filesystems.	Change determine_blacklist_path to accept a MountInfo pointer and use it when resolving blacklist paths. Change get_event_path to accept a MountInfo pointer and use it when resolving event paths for indexing items. Update default_event_handler construction and config-event handling to create mount_info_ earlier and pass it to all get_full_path/determine_blacklist_path call sites. Use mount_info_get_device_mount_point instead of get_device_root_mountpoint so mount resolution goes through the shared MountInfo instance.	src/daemon/src/core/default_event_handler.cpp src/daemon/src/utils/tools.c
Adjust tools API to be mount-aware and import mount_info definitions where needed.	Change get_full_path signature to take a MountInfo pointer and update its implementation accordingly. Include core/mount_info.h in tools.h to provide the MountInfo type to C callers. Update all callers in C++ code to match the new get_full_path signature.	src/daemon/include/utils/tools.h src/daemon/src/utils/tools.c src/daemon/src/core/default_event_handler.cpp

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• Now that get_full_path and determine_blacklist_path depend on a MountInfo*, consider adding an explicit null check or assertion on mount_info at their entry points (or in the caller) to avoid undefined behavior if mount_info_new() ever fails or a null is inadvertently passed.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- Now that `get_full_path` and `determine_blacklist_path` depend on a `MountInfo*`, consider adding an explicit null check or assertion on `mount_info` at their entry points (or in the caller) to avoid undefined behavior if `mount_info_new()` ever fails or a null is inadvertently passed.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[wangrong1069]

/merge

[deepin-bot]

TAG Bot

✅ Tag created successfully

📋 Tag Details

• Tag Name: 7.0.38
• Tag SHA: 45cd14b6028bc407e1ba762d203681f501e3d101
• Commit SHA: b08686b79ef7890b4f849e9eff8461df30e3f0a7
• Tag Message:

  Release deepin-anything 7.0.38
  
  

• Tagger:
  • Name: wangrong1069
• Distribution: unstable

[deepin-ci-robot]

deepin pr auto review

Git Diff 代码审查报告

总体评估

这次提交主要对 deepin-anything 项目进行了重构，主要改动包括：

1. 修改了路径获取逻辑，从直接调用系统函数改为通过 MountInfo 对象获取
2. 调整了 MountInfo 对象的初始化顺序
3. 修改了 systemd 服务配置，禁用了 ProtectKernelTunables 选项

下面从语法逻辑、代码质量、代码性能和代码安全四个方面进行详细审查。

语法逻辑审查

1. 函数签名变更一致性

   • get_full_path 函数签名从 char *get_full_path(const char *path) 变更为 char *get_full_path(MountInfo *mount_info, const char *path)
   • 所有调用该函数的地方都正确更新了，包括 determine_blacklist_path 和 get_event_path 函数
   • 评价：函数签名变更后所有调用点都正确更新，没有遗漏

2. MountInfo 对象初始化顺序

   • 在 default_event_handler 构造函数中，mount_info_ 的初始化从构造函数末尾移到了更早的位置
   • 原代码：

     // init indexing_items_
     spdlog::info("processing indexing_paths...");
     for (auto& origin_path : config_.indexing_paths) {
         std::string event_path_with_slash = get_event_path(origin_path, indexing_items_);
         // ...
     }
     // ...
     mount_info_ = mount_info_new();

   • 新代码：

     event_queue_ = g_async_queue_new();
     event_filter_thread_ = g_thread_new("event_filter", event_filter_thread_func, this);
     config_event_queue_ = g_async_queue_new();
     mount_info_ = mount_info_new();  // 提前初始化
     
     // init indexing_items_
     spdlog::info("processing indexing_paths...");
     for (auto& origin_path : config_.indexing_paths) {
         std::string event_path_with_slash = get_event_path(mount_info_, origin_path, indexing_items_);
         // ...
     }

   • 评价：这是一个必要的修改，因为现在 get_event_path 函数需要 mount_info_ 对象，所以必须在使用前初始化

代码质量审查

1. 资源管理

   • 在 tools.c 中，get_full_path 函数使用 mount_info_get_device_mount_point 获取挂载点，而不是原来的 get_device_root_mountpoint
   • 原代码中 get_device_root_mountpoint 返回的指针需要手动释放，而新代码中 mount_info_get_device_mount_point 返回的是 const gchar *，不需要手动释放
   • 评价：这是一个改进，减少了内存管理的复杂性，降低了内存泄漏的风险

2. 代码可读性

   • 在 systemd 服务配置文件中，添加了注释说明为什么禁用 ProtectKernelTunables：

     # need to write files in /sys/kernel/vfs_monitor dir
     # ProtectKernelTunables=true
     

   • 评价：添加注释提高了代码可读性，使维护者更容易理解为什么需要禁用这个安全特性

3. 错误处理

   • 在 get_full_path 函数中，如果 mount_info_get_device_mount_point 返回 NULL，函数会返回 NULL
   • 调用者正确处理了这种情况，例如在 get_event_path 函数中：

     if (event_path == nullptr) {
         spdlog::warn("Failed to get event path, use the origin path: {}", origin_path);
         event_path_str = origin_path;
     }

   • 评价：错误处理逻辑合理，提供了适当的回退机制和日志记录

代码性能审查

1. 缓存优化

   • MountInfo 对象在构造函数中创建一次，然后在多个函数调用中重复使用
   • 评价：这是一个良好的实践，避免了重复创建和销毁对象的开销

2. 函数调用开销

   • 新代码中，每次调用 get_full_path 都需要传递 mount_info 对象
   • 评价：这是一个必要的改动，因为现在路径解析依赖于 mount_info 对象中的信息。由于只传递指针，性能影响可以忽略不计

代码安全审查

1. systemd 安全配置

   • 禁用了 ProtectKernelTunables 选项，允许程序写入 /sys/kernel/vfs_monitor 目录
   • 评价：这是一个安全权衡，虽然降低了系统的安全性，但可能是程序功能所必需的。建议：
     • 评估是否可以限制程序只能访问 /sys/kernel/vfs_monitor 目录，而不是整个内核可调参数
     • 考虑使用其他机制（如文件系统权限）来限制对 /sys/kernel/vfs_monitor 的访问

2. 空指针检查

   • 在 get_full_path 函数中，没有检查 mount_info 参数是否为 NULL
   • 评价：这是一个潜在的安全问题，建议添加空指针检查：

     char *get_full_path(MountInfo *mount_info, const char *path)
     {
         if (!mount_info || !path) {
             return NULL;
         }
         // ...
     }

3. 路径遍历安全

   • get_full_path 函数使用 lstat 检查路径，这可以防止符号链接攻击
   • 评价：这是一个良好的安全实践

改进建议

1. 添加空指针检查

   • 在 get_full_path 函数中添加对 mount_info 和 path 参数的空指针检查

2. 考虑使用智能指针

   • 在 C++ 代码中，可以考虑使用智能指针管理 MountInfo 对象的生命周期，避免内存泄漏

3. 限制 systemd 安全降级范围

   • 评估是否可以使用 ReadWritePaths 选项限制程序只能访问特定的 /sys/kernel/vfs_monitor 目录，而不是完全禁用 ProtectKernelTunables

4. 添加单元测试

   • 为修改的函数添加单元测试，特别是 get_full_path 函数，确保它在各种情况下都能正确工作

5. 文档更新

   • 更新相关文档，说明为什么需要禁用 ProtectKernelTunables，以及可能的安全影响

总结

这次提交主要改进了路径解析逻辑，通过引入 MountInfo 对象来集中管理挂载点信息，这是一个良好的架构改进。代码质量整体较高，但在安全性方面还有改进空间，特别是添加空指针检查和评估 systemd 安全配置的影响。建议在合并前考虑上述改进建议。