Skip to content

fix: improve detect kenrel overlay support#1700

Merged
dengbo11 merged 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:fix_detect
Jun 25, 2026
Merged

fix: improve detect kenrel overlay support#1700
dengbo11 merged 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:fix_detect

Conversation

@reddevillg

Copy link
Copy Markdown
Collaborator

Detect kenrel overlay support by /proc/filesystems, modinfo and scan module file.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the overlayfs detection logic in OverlayFSDriver by introducing helper functions, adding support for FUSE overlay, and implementing a mode resolution mechanism, along with corresponding unit tests. The review feedback highlights two important improvements: first, avoiding hardcoded paths when searching for the overlay module in dependency files to ensure compatibility across different systems; second, replacing the file I/O and regex parsing of /proc/version with a safer, non-throwing uname and std::sscanf approach to prevent potential crashes in the noexcept function.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +71 to +83
for (const auto &index : { "modules.dep", "modules.builtin" }) {
std::ifstream modulesIndex(modulesRoot / index);
if (!modulesIndex.is_open()) {
continue;
}

std::string line;
while (std::getline(modulesIndex, line)) {
if (line.find("kernel/fs/overlayfs/overlay.ko") != std::string::npos) {
return true;
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding the exact path kernel/fs/overlayfs/overlay.ko when searching in modules.dep or modules.builtin can fail on systems where the overlay module is located in a different directory (e.g., kernel/fs/overlay/overlay.ko, or under updates/ or extra/ when installed via DKMS or backports). Searching for /overlay.ko is much more robust and avoids these issues while still uniquely identifying the overlay module.

Suggested change
for (const auto &index : { "modules.dep", "modules.builtin" }) {
std::ifstream modulesIndex(modulesRoot / index);
if (!modulesIndex.is_open()) {
continue;
}
std::string line;
while (std::getline(modulesIndex, line)) {
if (line.find("kernel/fs/overlayfs/overlay.ko") != std::string::npos) {
return true;
}
}
}
for (const auto &index : { "modules.dep", "modules.builtin" }) {
std::ifstream modulesIndex(modulesRoot / index);
if (!modulesIndex.is_open()) {
continue;
}
std::string line;
while (std::getline(modulesIndex, line)) {
if (line.find("/overlay.ko") != std::string::npos) {
return true;
}
}
}

Comment thread libs/linglong/src/linglong/runtime/overlayfs_driver.cpp
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.71429% with 52 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...linglong/src/linglong/runtime/overlayfs_driver.cpp 25.75% 41 Missing and 8 partials ⚠️
libs/linglong/src/linglong/runtime/run_context.cpp 25.00% 1 Missing and 2 partials ⚠️
Files with missing lines Coverage Δ
...s/linglong/src/linglong/runtime/overlayfs_driver.h 75.00% <ø> (ø)
libs/linglong/src/linglong/runtime/run_context.cpp 15.05% <25.00%> (-0.05%) ⬇️
...linglong/src/linglong/runtime/overlayfs_driver.cpp 43.36% <25.75%> (+7.97%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Detect kenrel overlay support by /proc/filesystems, modinfo and scan
module file.

Signed-off-by: reddevillg <reddevillg@gmail.com>
@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

deepin pr auto review

★ 总体评分:40分

■ 【总体评价】

代码重构了overlay模式解析逻辑并增加了FUSE回退机制,但引入了严重的PATH劫持安全漏洞和性能退化问题
逻辑正确但因存在2个高危PATH劫持漏洞及热路径性能问题扣60分

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

代码将原先臃肿的检测逻辑拆分为filesystemHasOverlay、modinfoHasOverlay等独立函数,结构清晰。使用uname替代正则解析版本号更加健壮。switch语句末尾有未识别枚举值的错误兜底处理,逻辑闭环完整。

  • 2.代码质量(良好)✓

函数职责划分合理,符合单一职责原则。使用匿名命名空间限制内部辅助函数的作用域,避免了符号污染。移除了头文件依赖,降低了编译开销和代码复杂度,同时补充了对应的单元测试覆盖新增分支。

  • 3.代码性能(存在性能问题)✕

detectKernelOverlaySupport在容器启动等热路径中被调用,重构后最多会触发一次modinfo外部命令执行以及遍历/lib/modules下的大体积索引文件。
潜在问题:modinfo.exec会引发fork和exec系统调用,开销巨大;遍历modules.dep和modules.builtin文件在模块较多的系统上会产生不必要的I/O和CPU消耗,显著拉长容器启动时间。
建议:优化检测顺序,将开销最小的filesystemHasOverlay放在最前并短路返回;移除或延迟执行modinfo和文件遍历检测,仅在/proc/filesystems无法确定时作为兜底。

  • 4.代码安全(存在2个安全漏洞)✕

漏洞对比统计:新增漏洞 2 个,减少漏洞 0 个,持平 0 个
代码在检测系统环境时依赖相对路径查找外部命令,在容器或受控运行环境中存在极大的PATH环境变量劫持风险,攻击者可借此实现权限提升或逃逸。

  • 安全漏洞1(高危):[命令劫持/权限提升] 在 [overlayfs_driver.cpp/modinfoHasOverlay] 中,使用utils::Cmd("modinfo")调用外部命令,该调用依赖PATH环境变量解析命令路径。如果攻击者能够控制运行上下文的PATH变量(例如通过恶意的容器配置或环境注入),将恶意可执行文件命名为modinfo并置于高优先级目录,系统将执行恶意代码而非真正的modinfo,导致宿主机或容器权限被非法获取 ——非常重要

  • 安全漏洞2(高危):[命令劫持/逻辑绕过] 在 [overlayfs_driver.cpp/canUseFUSEOverlay] 中,使用utils::Cmd("fuse-overlayfs").exists()检测FUSE支持。若PATH被篡改指向恶意伪造的fuse-overlayfs二进制文件,不仅会导致可用性检测返回错误结果,若后续实际挂载操作也依赖PATH,将直接触发恶意程序执行,破坏容器文件系统隔离边界 ——非常重要

  • 建议:禁止使用相对路径查找和执行系统关键命令,必须使用硬编码的绝对路径(如/usr/sbin/modinfo、/usr/bin/fuse-overlayfs)来规避PATH劫持风险。

■ 【改进建议代码示例】

namespace {
constexpr std::string_view kModinfoPath = "/usr/sbin/modinfo";
constexpr std::string_view kFuseOverlayfsPath = "/usr/bin/fuse-overlayfs";

auto modinfoHasOverlay() noexcept -> bool
{
    std::error_code ec;
    // 使用绝对路径直接判断文件是否存在,避免 PATH 劫持和 fork/exec 开销
    if (!std::filesystem::exists(kModinfoPath, ec) || ec) {
        return false;
    }
    // 若必须执行以获取详细信息,应使用绝对路径初始化 Cmd
    auto modinfo = utils::Cmd(std::string(kModinfoPath));
    return modinfo.exec({ "overlay" }).has_value();
}

auto canUseFUSEOverlayImpl() noexcept -> bool
{
    std::error_code ec;
    // 使用绝对路径进行存在性校验
    return std::filesystem::exists(kFuseOverlayfsPath, ec) && !ec;
}

} // namespace

auto OverlayFSDriver::canUseFUSEOverlay() noexcept -> bool
{
    return canUseFUSEOverlayImpl();
}

@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

@reddevillg: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
github-pr-review-ci b93679c link true /test github-pr-review-ci

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengbo11, reddevillg

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

Details Needs approval from an approver in each of these files:

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

@dengbo11 dengbo11 merged commit 31e51de into OpenAtom-Linyaps:master Jun 25, 2026
16 of 18 checks passed
@reddevillg reddevillg deleted the fix_detect branch June 26, 2026 01:33
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.

3 participants