Skip to content

refactor(mount): use memfd instead of pipe for password transfer#298

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
GongHeng2017:202605251357-master-fix
May 26, 2026
Merged

refactor(mount): use memfd instead of pipe for password transfer#298
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
GongHeng2017:202605251357-master-fix

Conversation

@GongHeng2017
Copy link
Copy Markdown
Contributor

Replace pipe-based password transmission with memfd_create for better security and efficiency. Remove Qt5 compatibility code including base64 encoding/decoding logic.

使用 memfd_create 替代管道传递密码,提升安全性和效率。
移除 Qt5 兼容代码,包括 base64 编解码逻辑。

Log: 使用memfd替代管道传递网络挂载密码,移除Qt5兼容代码
PMS: https://pms.uniontech.com/task-view-389921.html
Influence: 所有网络挂载密码传输方式从管道改为内存文件描述符,不再支持Qt5版本。

Replace pipe-based password transmission with memfd_create for
better security and efficiency. Remove Qt5 compatibility code
including base64 encoding/decoding logic.

使用 memfd_create 替代管道传递密码,提升安全性和效率。
移除 Qt5 兼容代码,包括 base64 编解码逻辑。

Log: 使用memfd替代管道传递网络挂载密码,移除Qt5兼容代码
PMS: https://pms.uniontech.com/task-view-389921.html
Influence: 所有网络挂载密码传输方式从管道改为内存文件描述符,不再支持Qt5版本。
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份 Git Diff 主要对 dnetworkmounter.cpp 进行了两项重大修改:

  1. 统一 Qt 版本逻辑:移除了 #if QT_VERSION < QT_VERSION_CHECK(6, 0, 0) 的条件编译,统一采用 Qt6 的处理逻辑(明文存储密码,挂载时通过 FD 传输)。
  2. 密码传输机制重构:将 preparePasswd 函数中基于匿名管道的文件描述符传递,重构为基于 memfd_create 的传递。

整体来看,使用 memfd_create 替代 pipe 是一个优秀的改进,因为它避免了管道缓冲区大小受限的潜在问题,且语义更清晰。但代码在安全性、健壮性和逻辑一致性上仍有一些需要改进的地方。

以下是详细的审查意见:

1. 代码安全

严重问题:内存中可能残留敏感信息(密码明文)
memfd_create 创建的内存区域和 QByteArray 在生命周期结束被销毁时,默认不会将内存清零。这意味着密码明文可能会残留在进程的堆内存或 memfd 的页缓存中,增加被恶意进程读取的风险(尤其是面对内存转储或未初始化内存漏洞时)。

  • 改进建议
    1. 写入 memfd 后,如果不再需要该数据,应使用 mmap 映射该内存并调用 explicit_bzeromemset_s 清零,然后再关闭 fd。
    2. 对于 QByteArray byteData,在数据写入 fd 后,应手动擦除其内部缓冲区(Qt 5.15+ / Qt 6 提供 QByteArray::fill() 或手动遍历赋值)。

2. 语法逻辑

潜在问题:memfd_create 在旧内核上的兼容性
memfd_create 系统调用是在 Linux 3.17 中引入的。如果目标系统运行在非常旧的内核上,memfd_create 会返回 ENOSYS。原先的 pipe 方案具有更好的内核兼容性。

  • 改进建议
    如果仍需支持 Linux 3.17 以下的内核,建议加上回退机制:
    int fd = memfd_create("DBusFD", MFD_CLOEXEC);
    if (fd < 0) {
        if (errno == ENOSYS) {
            // Fallback to pipe logic
        } else {
            qCritical() << "Failed to create memfd for data transfer:" << strerror(errno);
            return QVariant("");
        }
    }

逻辑一致性问题:loginPasswd 的返回格式与 preparePasswd 不匹配
loginPasswd 函数中(第 174 行),当查询到密码时,现在的逻辑是 passwd.insert(kLoginPasswd, QString(pwd));,即直接插入明文字符串
然而,在 mountWithSavedInfos 函数中(第 617 行),它取出该值并传给 preparePasswdlogin.value(kLoginPasswd, "").toString()
如果 loginPasswd 返回的确实是明文,那么这里的逻辑是自洽的。但需确保底层的守护进程和本地存储读取逻辑都已经同步去除了 Base64 的编解码。如果底层存储仍然是 Base64,这里就会导致密码传递错误。

  • 改进建议:仔细核对 loginPasswd 获取到的 pwd 是否真的是明文。如果是 Base64,则需要在此处先解码再传给 preparePasswd

3. 代码性能

合理改进:移除 Base64 编解码
移除了 Qt5 下的 Base64 编解码逻辑,这不仅减少了 CPU 的计算开销,也避免了 Base64 编码带来的约 33% 的数据体积膨胀,对性能和数据传输效率都是正向的。

4. 代码质量

问题 1:缺少 memfd_create 的头文件保护
memfd_create 并不是 POSIX 标准函数,而是 Linux 特有的。在某些 GLIBC 版本中,直接 #include <sys/mman.h> 可能无法声明该函数,需要定义宏 #define _GNU_SOURCE

  • 改进建议:确保在包含 <sys/mman.h> 之前,在文件开头或项目编译定义中加入了 #define _GNU_SOURCE

问题 2:错误日志不够详细
preparePasswd 函数中,发生系统调用失败时,没有打印 errno 对应的错误字符串,这会给线上问题排查带来困难。

  • 改进建议:在 qCritical 中加入 strerror(errno)

改进后的代码示例

综合以上意见,preparePasswd 函数的改进版本如下:

// 确保在文件顶部或 CMake 中定义了 _GNU_SOURCE
#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif

#include <sys/mman.h>
// ... 其他头文件

static QVariant preparePasswd(const QString &passwd)
{
    if (passwd.isEmpty()) {
        return QVariant("");
    }

    int fd = memfd_create("DBusFD", MFD_CLOEXEC);
    if (fd < 0) {
        // 增加错误原因输出,并考虑兼容性回退
        qCritical() << "Failed to create memfd for data transfer:" << strerror(errno);
        return QVariant("");
    }

    QByteArray byteData = passwd.toUtf8();
    ssize_t written = ::write(fd, byteData.constData(), byteData.size());
    
    // 安全性改进:写入后擦除 QByteArray 中的敏感数据
    byteData.fill('\0'); 
    
    if (written < 0 || static_cast<ssize_t>(byteData.size()) != written) {
        qCritical() << "Failed to write data to memfd:" << strerror(errno);
        ::close(fd);
        return QVariant("");
    }

    if (lseek(fd, 0, SEEK_SET) < 0) {
        qCritical() << "Failed to seek memfd to beginning:" << strerror(errno);
        ::close(fd);
        return QVariant("");
    }

    // 安全性改进:擦除 memfd 中的敏感数据
    // 注意:由于 dbus 底层读取是异步的,直接在这里清零 memfd 可能会导致 dbus 侧读到全零数据。
    // 如果要彻底清零,需要在 DBus 调用完成后的回调中进行,或者依赖 fd 关闭后内核对内存的回收。
    // 因此,这里只对 QByteArray 进行了 fill('\0') 清零。

    QDBusUnixFileDescriptor dbusFd;
    dbusFd.giveFileDescriptor(fd);
    // fd 的所有权已转移给 dbusFd,不需要再手动 close(fd)

    return QVariant::fromValue(dbusFd);
}

特别提醒关于 memfd 清零的时序问题
如代码注释中所述,QDBusUnixFileDescriptor 只是把 fd 的副本传递给了 DBus 底层,真正的读取发生在 DBus 后台线程中。如果在 giveFileDescriptor 之后立刻用 mmap 清零 memfd,会导致 DBus 读到空密码。因此,对于 memfd 内存的敏感数据清理,最安全的时机是在 mountWithUserInput 整个挂载任务完成后的回调中处理,或者确保系统没有交换分区,并依赖进程退出时的内存回收机制。对于 QByteArray 则可以立即清零,因为它已经被 write 系统调用复制到了内核空间。

@wangrong1069
Copy link
Copy Markdown
Contributor

+1

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017, Johnson-zs, 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:

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

@GongHeng2017
Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot Bot merged commit da9ba19 into linuxdeepin:master May 26, 2026
21 checks passed
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.

4 participants