Skip to content

fix: change the propagation type of the root mount to shared#1686

Closed
reddevillg wants to merge 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:fix_propagation
Closed

fix: change the propagation type of the root mount to shared#1686
reddevillg wants to merge 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:fix_propagation

Conversation

@reddevillg

Copy link
Copy Markdown
Collaborator

The mount propagation type will be private,slave if a new mount namespace is creted by clone() and CLONE_NEWUSER is also used. Change it to shared,slave.

@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 introduces a mount system call in runInNamespaceEntry to set the propagation type of the root directory to shared, along with the necessary header inclusion. The review feedback correctly points out that the return value of the mount call should be checked and handled to avoid silent failures if the operation fails.

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 thread libs/utils/src/linglong/utils/namespace.cpp Outdated
The mount propagation type will be private,slave if a new mount
namespace is creted by clone() and CLONE_NEWUSER is also used.
Change it to shared,slave.

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

Copy link
Copy Markdown
Collaborator

deepin pr auto review

★ 总体评分:20分

■ 【总体评价】

代码实现了根目录挂载传播类型修改,但存在严重的安全隔离破坏漏洞
逻辑正确但因破坏容器挂载命名空间隔离导致严重安全漏洞扣80分

■ 【详细分析】

  • 1.语法逻辑(存在严重错误)✕
    runInNamespaceEntry 函数中,使用 mount("", "/", "", MS_SHARED | MS_REC, NULL) 修改根目录传播类型在C/C++语法层面没有编译错误,但在容器/命名空间隔离的业务逻辑层面属于严重错误。MS_SHARED 会使该命名空间内的挂载事件与宿主机或其他对等命名空间双向同步,完全违背了创建独立命名空间进行隔离的初衷。
    潜在问题:破坏命名空间隔离边界;可能导致宿主机和容器挂载状态互相污染甚至引发文件系统死锁。
    建议:将 MS_SHARED 修改为 MS_SLAVEMS_PRIVATE,以确保挂载事件的正确单向隔离或完全隔离。
  • 2.代码质量(良好)✓
    引入了必要的 <sys/mount.h> 头文件,使用了 LogW 进行错误日志记录,并通过 common::error::errorString(errno) 将底层错误码转换为可读字符串,符合现有的代码规范和错误处理模式。
    建议:建议增加注释说明为什么需要在此处修改挂载传播类型以及选择特定传播类型的原因,由于此处选型存在严重问题,更凸显注释的必要性。
  • 3.代码性能(高效)✓
    仅执行一次 mount 系统调用,不涉及循环、内存分配或复杂计算,系统开销极低,无性能问题。
    建议:无
  • 4.代码安全(存在 1 个安全漏洞(严重1个))✕
    漏洞对比统计:新增漏洞 1 个,减少漏洞 0 个,持平 0 个
    在命名空间初始化阶段将根目录设置为 shared 会导致容器内的挂载操作直接泄漏到宿主机,破坏了 UOS 系统中 linyaps 容器的核心安全边界。

  • 安全漏洞1(严重):容器逃逸/挂载命名空间隔离破坏 在 runInNamespaceEntry 函数中,使用 MS_SHARED | MS_REC 标志将根目录 / 的挂载传播类型设置为共享。这导致容器内后续的任何挂载或卸载操作都会传播到宿主机命名空间,攻击者可在容器内通过挂载宿主机磁盘、覆盖宿主机关键文件(如 /etc/shadow/etc/sudoers)的方式实现本地权限提升或拒绝服务攻击 ——非常重要

  • 建议:必须将 MS_SHARED 替换为 MS_PRIVATE(完全隔离)或 MS_SLAVE(仅接收宿主机挂载事件但不向外传播),以修复隔离破坏漏洞。

■ 【改进建议代码示例】

diff --git a/libs/utils/src/linglong/utils/namespace.cpp b/libs/utils/src/linglong/utils/namespace.cpp
index 463bce464..123456789 100644
--- a/libs/utils/src/linglong/utils/namespace.cpp
+++ b/libs/utils/src/linglong/utils/namespace.cpp
@@ -313,8 +313,9 @@ int runInNamespaceEntry(void *args)
         }
     }
 
-    if (mount("", "/", "", MS_SHARED | MS_REC, NULL) == -1) {
-        LogW("failed to change propagation type of / to shared: {}",
+    // 使用 MS_PRIVATE 确保容器挂载事件不会泄漏到宿主机,保障命名空间隔离安全
+    if (mount("", "/", "", MS_PRIVATE | MS_REC, NULL) == -1) {
+        LogW("failed to change propagation type of / to private: {}",
              common::error::errorString(errno));
     }
 

@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 fbbd926 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.

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
libs/utils/src/linglong/utils/namespace.cpp 0.00% 2 Missing ⚠️
Files with missing lines Coverage Δ
libs/utils/src/linglong/utils/namespace.cpp 3.21% <0.00%> (-0.03%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@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

@reddevillg reddevillg marked this pull request as draft June 24, 2026 09:26
@reddevillg reddevillg closed this Jun 24, 2026
@reddevillg reddevillg deleted the fix_propagation branch June 25, 2026 01:34
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