Skip to content

feat(storage): add MMC storage vendor identification via sysfs#702

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
add-uos:master
Jul 7, 2026
Merged

feat(storage): add MMC storage vendor identification via sysfs#702
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
add-uos:master

Conversation

@add-uos

@add-uos add-uos commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Read manfid/oemid/name from sysfs for MMC storage devices and resolve vendor names using MANFID/OEMID lookup tables. Add safeReadSysFsFile() utility with path traversal protection.

Log: 添加MMC存储设备厂商识别功能,通过sysfs读取manfid/oemid识别厂商
PMS: BUG-368319
Influence: MMC/eMMC存储设备的厂商信息不再为空,可正确显示对应厂商名称

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @add-uos, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

lzwind
lzwind previously approved these changes Jul 7, 2026
Read manfid/oemid/name from sysfs for MMC storage devices and resolve
vendor names using MANFID/OEMID lookup tables. Add safeReadSysFsFile()
utility with path traversal protection.

Log: 添加MMC存储设备厂商识别功能,通过sysfs读取manfid/oemid识别厂商
PMS: BUG-368319
Influence: MMC/eMMC存储设备的厂商信息不再为空,可正确显示对应厂商名称
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:88分

■ 【总体评价】

代码实现了MMC设备厂商名和设备名的补充读取,但存在覆盖有效数据的逻辑缺陷
逻辑正确但因空值覆盖风险扣12分

■ 【详细分析】

  • 1.语法逻辑(存在错误)✕

DeviceStorage::setHwinfoInfo 函数中,调用 Common::safeReadSysFsFile 获取 namemanfIdRawoemIdRaw 时,仅通过 err.isEmpty() 判断是否成功。如果 sysfs 下的对应文件存在但内容为空(例如某些异常或未完全初始化的 MMC 设备),safeReadSysFsFile 会成功返回空字符串且 err 为空。此时 m_Name = name;m_Vendor = getManfName(manfIdRaw); 会用空字符串覆盖掉 hwinfo 前面已经解析并赋值的有效设备名和厂商名,导致显示信息丢失。
潜在问题:空字符串覆盖有效设备信息导致前端显示空白;manfid 为空时查表返回空字符串同样覆盖 Vendor
建议:在赋值前增加对读取内容非空的判断,如 if (err.isEmpty() && !name.isEmpty())

  • 2.代码质量(符合规范)✓

代码结构清晰,查表逻辑封装合理,safeReadSysFsFile 函数注释完整。MANFID_TABLEOEMID_TABLE 使用静态常量 QMap 定义,符合 C++ 规范。getManfNamegetOemName 方法虽然未修改成员变量,但作为普通成员函数不影响当前编译和基本使用。
建议:将 DeviceStorage::getManfNameDeviceStorage::getOemName 声明和实现中加上 const 修饰符,以符合 C++ 最佳实践并便于编译器优化

  • 3.代码性能(高效)✓

safeReadSysFsFile 中限制了最大读取长度为 64 字节,避免了读取大文件时的内存浪费和阻塞。使用 QMap 进行十几条记录的 ID 查表,时间复杂度为 O(log N),在当前数据量下性能开销极低,完全满足设备枚举时的性能要求。
建议:无需优化,当前性能表现良好

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码在 Common::safeReadSysFsFile 中展现了较高的安全意识。通过多层防御机制有效阻止了路径穿越:首先检查 baseDir 必须以 /sys/ 开头且不含 ..;其次检查 fileName 不含路径分隔符和 ..;最关键的是使用 QFileInfo::canonicalFilePath() 解析符号链接后,再次校验最终绝对路径是否严格在 /sys/ 目录下。这彻底切断了通过符号链接指向 /etc/shadow 等敏感文件的攻击面。错误信息通过引用参数返回而非直接打印日志,避免了输入内容导致的日志注入。QString::arg() 替代了 C 风格的格式化函数,杜绝了格式化字符串漏洞。

  • 建议:保持现有的安全编码习惯,canonicalFilePath() 是防范符号链接攻击的标准且有效的手段

■ 【改进建议代码示例】

// DeviceStorage.cpp:修复空值覆盖逻辑缺陷
if (m_Driver.contains("mmcblk")) {
    QString sysfsDeviceLink = mapInfo.value("SysFS Device Link");

    if (!sysfsDeviceLink.isEmpty())
    {
        if (!sysfsDeviceLink.startsWith("/sys/"))
        {
            sysfsDeviceLink = "/sys" + sysfsDeviceLink;
        }

        QString err;
        QString name = Common::safeReadSysFsFile(sysfsDeviceLink, "name", err);
        // 增加对读取内容非空的校验,防止空字符串覆盖 hwinfo 已解析的有效数据
        if (err.isEmpty() && !name.isEmpty()) {
            m_Name = name;
        }

        QString manfIdRaw = Common::safeReadSysFsFile(sysfsDeviceLink, "manfid", err);
        if (err.isEmpty() && !manfIdRaw.isEmpty()) {
            QString manfName = getManfName(manfIdRaw);
            if (!manfName.isEmpty()) {
                m_Vendor = manfName;
            }
        }

        QString oemIdRaw = Common::safeReadSysFsFile(sysfsDeviceLink, "oemid", err);
        if (err.isEmpty() && !oemIdRaw.isEmpty()) {
            if (m_Vendor.isEmpty()) {
                m_Vendor = getOemName(oemIdRaw);
            }
        }
    }
}

// DeviceStorage.h:建议补充 const 修饰符
// QString getManfName(const QString &rawManfId) const;
// QString getOemName(const QString &rawOemId) const;

// DeviceStorage.cpp:建议补充 const 修饰符
// QString DeviceStorage::getManfName(const QString &rawManfId) const
// QString DeviceStorage::getOemName(const QString &rawOemId) const

@add-uos

add-uos commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot

deepin-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

This pr cannot be merged! (status: blocked)

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: add-uos, lzwind

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

@add-uos

add-uos commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot Bot merged commit f373005 into linuxdeepin:master Jul 7, 2026
23 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.

3 participants