Repository navigation
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideThe PR fixes multi-interface USB authorization path corruption by preserving original SysFS paths, updates the affected expectations, and adds extensive isolated unit coverage around EnableUtils behavior and error branches. Sequence diagram for preserving USB interface SysFS pathssequenceDiagram
participant Caller
participant EnableUtils
participant SysFS
Caller->>EnableUtils: disableOutDevice(info)
EnableUtils->>EnableUtils: isSafeSysfsPath(path)
EnableUtils->>SysFS: write authorized using original path
SysFS-->>EnableUtils: write result
EnableUtils-->>Caller: disable result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ❌评价: 存在逻辑缺陷 ❌ 不通过 潜在问题: 建议: ['将3个测试用例的期望路径从归一化值修改为原始输入值(1-0→1-2, input0→input6, link0→link6)', '更新第405行注释,移除关于归一化的描述,改为说明路径保持原值'] 2. 代码质量 ✅评价: 代码结构清晰,注释完整 ✅ 通过 潜在问题: 建议: ['在 ut_qtSym 中添加 dlopen 和 dlsym 的空指针检查,失败时输出错误信息并返回 nullptr', '在 TearDown 中集中重置所有全局桩状态变量,保证每个测试用例的清洁状态'] 3. 代码性能 ✅评价: 性能良好,资源使用合理 ✅ 通过 潜在问题: 建议: [] 4. 代码安全 🔒评价: 存在0个安全漏洞 ✅ 通过
安全漏洞详情: 建议: [] 💡 改进建议代码示例// 修复1: 更新测试期望值(ut_enableutils.cpp)
// disableOutDevice_usbInDb_writesAuthorizedFileAndUpdatesPath
// 将第412行:
// EXPECT_EQ(mgr->authorizedPath("ut-serial-xyz"), QStringLiteral("/devices/ut_t27/usb1/1-0"));
// 改为:
// EXPECT_EQ(mgr->authorizedPath("ut-serial-xyz"), QStringLiteral("/devices/ut_t27/usb1/1-2"));
// 修复2: 更新 MD5 分支测试期望值(第800行)
// EXPECT_EQ(mgr->authorizedPath(expectUid), QStringLiteral("/devices/ut_t27/md5/input0"));
// 改为:
// EXPECT_EQ(mgr->authorizedPath(expectUid), QStringLiteral("/devices/ut_t27/md5/input6"));
// 修复3: 更新 Device Link 分支测试期望值(第827行)
// EXPECT_EQ(mgr->authorizedPath(expectUid), QStringLiteral("/devices/ut_t27/link0"));
// 改为:
// EXPECT_EQ(mgr->authorizedPath(expectUid), QStringLiteral("/devices/ut_t27/link6"));
// 修复4: 更新过时注释(第405行)
// // Act: SysFS ID 尾号 2 会被规范化替换为 0
// 改为:
// // Act: SysFS ID 路径保持原值,不再归一化尾号
// 修复5: ut_qtSym 添加空指针检查(第91-95行)
static void *ut_qtSym(const char *mangled)
{
static void *handle = dlopen("libQt6Core.so.6", RTLD_LAZY | RTLD_GLOBAL);
if (!handle) {
qWarning("ut_qtSym: dlopen failed: %s", dlerror());
return nullptr;
}
void *sym = dlsym(handle, mangled);
if (!sym) {
qWarning("ut_qtSym: dlsym failed for %s: %s", mangled, dlerror());
}
return sym;
}
// 修复6: TearDown 中重置全局桩状态(第38-39行)
void TearDown() override
{
ut_enableNetworkByDBus_ret = false;
ut_enableNetworkByDBus_calls = 0;
ut_enableNetworkByDBus_args.clear();
ut_enableNetworkByIoctl_ret = false;
ut_enableNetworkByIoctl_calls = 0;
ut_ioctlOperate_ret = false;
ut_ioctlOperate_calls = 0;
ut_ioctlOperate_args.clear();
ut_isSafeSysfs_ret = false;
ut_isSafeSysfs_calls = 0;
ut_ifaceValid_ret = false;
ut_ifaceValid_calls = 0;
ut_ifaceValid_seq.clear();
ut_doCall_calls = 0;
ut_doCall_seq.clear();
ut_qfileOpen_ret = false;
ut_qfileOpen_calls = 0;
ut_qfileWrites.clear();
ut_qfileExists_ret = false;
ut_qfileExists_calls = 0;
ut_socket_fd = -1;
ut_socketOpen_calls = 0;
ut_ioctlGiFlags_ret = 0;
ut_ioctlSiFlags_ret = 0;
ut_ioctlSiFlags_seen = -1;
}本报告由 AI 代码审查工具自动生成 |
1. Root cause: path.replace(QRegularExpression("[1-9]$"), "0")
blindly zeroed the last digit of SysFS paths, corrupting both
USB device port numbers and interface numbers
2. Fix: remove the normalization line so SysFS paths retain their
original values for authorized file access
3. Impact: multi-interface USB devices no longer have their authorized
paths overwritten; single-interface behavior unchanged
Log: fix USB device authorized path corruption from path normalization
Influence:
1. Test disabling USB devices with multi-interface configurations
2. Verify authorized file paths match actual SysFS paths
3. Test enabling/disabling single-interface USB devices for regression
fix: 移除 disableOutDevice 中错误的路径归一化
1. 根因:path.replace(QRegularExpression("[1-9]$"), "0") 粗暴地将
SysFS 路径末位数字归零,同时破坏了设备端口号和接口号
2. 方案:移除归一化行,使 SysFS 路径保持原值用于 authorized
文件访问
3. 影响:多接口 USB 设备不再出现 authorized 路径相互覆盖;
单接口设备行为不变
Log: 修复 USB 设备 authorized 路径因归一化导致被覆盖的问题
Influence:
1. 测试多接口 USB 设备的禁用功能
2. 验证 authorized 文件路径与实际 SysFS 路径一致
3. 测试单接口 USB 设备的启用/禁用以检查回归
895076e to
90d39bd
Compare
Root Cause Analysis
EnableUtils::disableOutDeviceusedpath.replace(QRegularExpression("[1-9]$"), "0")to normalize the last digit of SysFS paths to zero. This regex does not distinguish between USB device port numbers (e.g.,1-2) and interface numbers (e.g.,1-2:1.3), corrupting both. Multi-interface USB devices had their authorized file paths collapsed to the same value, causing database records to overwrite each other. Thedevelop/eaglebranch already removed this line in commitd0423190(PMS: BUG-377327), confirming this is a defect.Fix
Removed the
path.replace(QRegularExpression("[1-9]$"), "0");line fromenableutils.cpp:101. SysFS paths now retain their original values for authorized file access. Updated 3 pinning test case expectations inut_enableutils.cppto match the corrected behavior.Change Safety Assessment
Code Safety
develop/eaglefix.isSafeSysfsPathvalidation (3-layer: null check,..rejection, canonical path prefix/sys/) remains intact, so path traversal protection is unaffected.Business Impact Scope
Affected module: USB device enable/disable control (
deepin-devicecontrol). User scenarios: disabling multi-interface USB devices will now correctly target each interface's authorized file. Single-interface device behavior is unchanged since the path was already correct for interface 0.Verification Suggestion
Test disabling a multi-interface USB device and verify each interface's
/sys/<path>/authorizedfile is correctly written. Regression-test single-interface USB device enable/disable.根因分析
EnableUtils::disableOutDevice使用path.replace(QRegularExpression("[1-9]$"), "0")将 SysFS 路径末位数字归零。该正则不区分 USB 设备端口号(如1-2)和接口号(如1-2:1.3),同时破坏两者。多接口 USB 设备的 authorized 文件路径被归一化为相同值,导致数据库记录相互覆盖。develop/eagle分支已在 commitd0423190(PMS: BUG-377327)中移除此行,确认其为缺陷。修复方案
移除
enableutils.cpp:101的path.replace(QRegularExpression("[1-9]$"), "0");行,使 SysFS 路径保持原值用于 authorized 文件访问。同步更新ut_enableutils.cpp中 3 个 pinning 用例的预期值。改动安全评估
代码安全评估
develop/eagle修复方向一致。isSafeSysfsPath三层校验(空值检查、..拒绝、canonical path 以/sys/开头)完整保留,路径穿越防护不受影响。业务影响范围
受影响模块:USB 设备启用/禁用控制(
deepin-devicecontrol)。用户场景:禁用多接口 USB 设备时将正确 targeting 各接口的 authorized 文件。单接口设备行为不变(路径本身已为 interface 0)。验证建议
测试禁用多接口 USB 设备,验证各接口
/sys/<path>/authorized文件正确写入。回归测试单接口 USB 设备的启用/禁用。Summary by Sourcery
Remove incorrect SysFS path normalization to correctly disable multi-interface USB devices without changing existing path-safety validation.
Bug Fixes:
Enhancements:
Tests: