Skip to content

fix(devicecontrol): rebind USB HID driver on device enable - #731

Open
GongHeng2017 wants to merge 1 commit into
linuxdeepin:develop/eaglefrom
GongHeng2017:agent/pms-bug-bot/709e439c
Open

fix(devicecontrol): rebind USB HID driver on device enable#731
GongHeng2017 wants to merge 1 commit into
linuxdeepin:develop/eaglefrom
GongHeng2017:agent/pms-bug-bot/709e439c

Conversation

@GongHeng2017

@GongHeng2017 GongHeng2017 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

fix(devicecontrol): rebind USB HID driver on device enable

PMS: BUG-341577 · 目标分支 develop/eagle · 基线 ccd4ab90

问题现象

USB 鼠标在设备管理器中右键「禁用」后,右键「启用」仍无法使用,必须物理拔插才恢复。必现(飞腾 D3000M 平台)。

根因分析(强根因)

禁用后恢复 USB 鼠标的唯一代码路径是 ControlInterface::authorizedEnable 的启用分支(controlinterface.cpp 原三步序列)。该序列仅做「/sys/.../authorized 就地翻转」:

  • 第一步给接口授权后,第二步立即对父设备去授权——直接抹掉第一步(整设备去授权会解绑所有接口);
  • 第二步去授权与第三步重新授权紧邻,无 settle
  • 全程无显式的 HID/输入驱动重新绑定或重新探测(无 unbind/bindudevadm trigger)。

该就地翻转不等价于物理拔插的「断开—重连 + udev add」,usbhid 未被可靠重新绑定、输入节点未重建,故启用后鼠标无响应。

关键证据(file:line,基线 develop/eagle@ccd4ab90):

  • controlinterface.cpp:361 — 禁用向接口 authorized0(解绑 usbhid)。
  • controlinterface.cpp:341 / 350 / 356 — 启用三步就地翻转;:350 抹掉 :341,无 settle。
  • controlinterface.cpp:365 — 启用仍 return true,前端据此标记「已启用」(UI 与实际不符)。
  • controlinterface.cpp:372-376 — 同文件旁证:removeEnable/sys/bus/pci/rescan 做真正重新枚举,authorizedEnable 缺等价机制。

排除项::103 sPath = path480a9ef8/Bug 316525 引入)破坏的是「换 USB 口」回查,与本 bug(同口禁用-启用)无关,为独立共存 bug,不在本次修复范围。

修复方案

重构 authorizedEnable 启用分支为「断开—重连」等价序列(仅改该函数内部,签名不变):

  1. 父设备去授权(写 0)——整设备去授权,解绑所有接口驱动、销毁输入节点(断开等价);
  2. settleQThread::msleep(100))——等待内核完成去授权清理,避免去授权-重新授权竞态;
  3. 父设备重新授权(写 1)——整设备重新授权,重新枚举并探测接口驱动(重连等价);
  4. 显式重新授权接口(写 1)——禁用的直接逆操作,确保接口已授权(sysfs 条目在重新枚举后重建,重新打开);
  5. udevadm trigger --action=add --syspath=...——模拟物理拔插产生的 udev add 事件,确保输入设备节点重建。

移除了被自身抹掉的旧第一步(接口写 1 被随后的父设备去授权覆盖)。

改动安全评估

低风险。blame 显示目标行全部归因于原始引入提交 c0f83b6f(非既有 bug 修复,无撤销风险);authorizedEnable 唯一调用者是同文件 enable():109),签名不变、返回值语义不变,调用者不受负面影响。新增依赖为 Qt/系统标准设施(QThreadudevadm)。

验证建议

建议在复现机(飞腾 D3000M)验证:禁用 USB 鼠标 → 软件启用 → 滚动/点击可用(无需拔插)。可对照 dmesg/udevadm monitor/ls /dev/input/ 确认启用后输入节点重建。

Summary by Sourcery

Ensure USB HID devices are properly reinitialized when re-enabled via device manager by making the authorization sequence equivalent to a physical unplug-replug cycle.

Bug Fixes:

  • Fix USB mouse remaining unusable after being disabled and re-enabled in the device manager by reliably rebinding HID drivers and recreating input nodes.

Enhancements:

  • Refine the USB device authorization workflow to deauthorize and reauthorize the parent device with a settle delay, explicitly reauthorize the interface, and trigger udev add events so input devices are consistently restored.

Restructure authorizedEnable to mirror a physical replug: deauthorize
the parent USB device, settle for teardown, reauthorize it, explicitly
reauthorize the interface, then trigger udev to rebuild input devices.

重构 authorizedEnable 启用流程使其等价物理拔插:父设备去授权后等待
内核清理完成再重新授权,并显式重新授权接口、触发 udev 重建输入设备
节点,修复 USB 鼠标禁用后启用仍无响应需重新拔插的问题。

Log: 修复USB鼠标禁用后启用仍无法使用需重新拔插的问题
PMS: BUG-341577
Influence: USB外设禁用后启用可正常恢复使用,无需物理拔插

@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 @GongHeng2017, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017

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

@sourcery-ai

sourcery-ai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Reviewer's Guide

Refactors the USB device enable flow in ControlInterface::authorizedEnable to emulate a full disconnect–reconnect cycle so that USB HID drivers and input nodes are reliably rebound when a device is re-enabled.

Sequence diagram for updated USB device enable flow in authorizedEnable

sequenceDiagram
    participant ControlInterface
    participant SysfsParent as parent_authorized
    participant SysfsInterface as interface_authorized
    participant Udevadm
    participant Udevd

    ControlInterface->>SysfsParent: QFile::write("0") // parent /authorized
    ControlInterface->>ControlInterface: QThread::msleep(100)
    ControlInterface->>SysfsParent: QFile::write("1") // parent /authorized
    ControlInterface->>SysfsInterface: QFile::write("1") // interface /authorized
    ControlInterface->>Udevadm: QProcess::execute("udevadm trigger --action=add --syspath=/sys/...")
    Udevadm->>Udevd: add event for USB device
    Udevd-->>SysfsInterface: recreate input device nodes
    ControlInterface-->>ControlInterface: EnableSqlManager::removeDataFromAuthorizedTable(unique_id)
Loading

File-Level Changes

Change Details Files
Refactor the USB device authorization enable flow to perform a full parent deauthorize/re-authorize cycle with a settle and explicit interface re-authorization, plus a udev add trigger, to mimic physical replug behavior.
  • Remove the previous interface-only authorized write step that was immediately undone by parent deauthorization.
  • Change the enable sequence to first write 0 to the parent device’s authorized sysfs entry to fully deauthorize the device and unbind interface drivers.
  • Insert a 100 ms QThread::msleep settle period after deauthorization to let the kernel finish cleanup and avoid races.
  • Re-open and write 1 to the parent device’s authorized sysfs entry to re-authorize and re-enumerate the USB device.
  • Re-open and write 1 to the interface-level authorized file to explicitly re-authorize the interface after re-enumeration.
  • Execute udevadm trigger --action=add --syspath=/sys<parent-path> via QProcess to force udev to re-handle add events and rebuild input device nodes.
deepin-devicemanager-server/deepin-devicecontrol/src/controlinterface.cpp
Introduce QThread usage needed for the settle delay in the enable flow.
  • Add #include <QThread> to controlinterface.cpp to support msleep in the new enable sequence.
deepin-devicemanager-server/deepin-devicecontrol/src/controlinterface.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:50分

■ 【总体评价】

代码通过模拟物理拔插修复了USB鼠标无响应问题,但存在主线程阻塞和路径遍历等严重缺陷
逻辑正确但因安全漏洞及严重性能问题扣50分

■ 【详细分析】

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

在 ControlInterface::authorizedEnable 函数的第四步接口重新授权逻辑中,使用 if (file.open(QIODevice::ReadWrite)) 进行包裹,当文件打开失败时,没有执行 return false,而是静默跳过并继续执行后续的数据库删除操作。这与第一步和第三步中文件打开失败直接返回 false 的逻辑不一致。
潜在问题:接口授权失败但数据库中的禁用记录被删除,导致系统状态与数据库状态不一致,后续可能无法再次触发正确的启用流程
建议:在 if 分支后增加 else 分支并执行 return false,确保任一环节失败均能正确回滚并终止流程

  • 2.代码质量(存在严重问题)✕

在 ControlInterface::authorizedEnable 函数的第二步中,直接调用了 QThread::msleep(100) 强制阻塞当前线程。由于该类属于 DBus 服务端接口,这种做法会直接冻结整个 DBus 服务的事件循环,导致在此期间服务无法响应任何其他客户端的请求。
潜在问题:主线程阻塞导致服务假死,违反了 Qt 事件驱动编程的核心规范,降低了系统服务的整体健壮性
建议:移除 QThread::msleep 调用,改用异步状态机设计,或者通过监听内核发出的 udev 事件来判断去授权清理是否完成

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

代码在第五步使用了 QProcess::execute 同步执行 udevadm trigger 命令,该函数会阻塞直到外部进程结束。结合前文的 QThread::msleep(100),整个启用流程的耗时被严重放大,DBus 调用延迟至少在 100ms 加上进程启动执行时间之上。
潜在问题:同步系统调用极大增加了接口响应延迟,在频繁操作或底层子系统负载高时,可能触发 DBus 调用超时
建议:将 QProcess::execute 替换为 QProcess::startDetached 实现非阻塞触发,或者重构为异步流程通过信号槽处理后续逻辑

  • 4.代码安全(存在 2 个安全漏洞(中危1个,低危1个))✕

漏洞对比统计:新增漏洞 2 个,减少漏洞 0 个,持平 0 个
外部传入的 path 参数未经任何校验直接用于构造系统文件路径和命令参数,存在越权操作和参数注入风险

  • 安全漏洞1(中危):路径遍历 在 ControlInterface::authorizedEnable 函数中,path 参数来自外部 DBus 调用,未经过合法性校验即通过 QFileInfo::path() 处理并拼接 "/sys" 前缀写入文件。如果攻击者构造包含 "../" 的恶意路径(如 "/devices/../../etc/crontab"),可能越界写入非预期的 sysfs 节点或引发内核异常行为,导致系统配置被篡改或拒绝服务 ——非常重要

  • 安全漏洞2(低危):参数注入 在 ControlInterface::authorizedEnable 函数中,未经校验的 pop 变量被直接拼接至 udevadm 的 --syspath 参数中。虽然 QProcess 的 QStringList 参数传递机制避免了 Shell 注入,但若 pop 包含空格或特殊字符,会导致 udevadm 接收到畸形路径参数,引发非预期的设备枚举行为或产生大量错误日志 ——非常重要

  • 建议:在函数入口处对 path 参数进行严格的正则白名单校验(如限制仅允许包含字母、数字、冒号、点、斜杠),确保其符合 sysfs 标准路径格式;在调用 udevadm 前检查拼接后的绝对路径是否以预期的 /sys/devices/ 为前缀

■ 【改进建议代码示例】

diff --git a/deepin-devicemanager-server/deepin-devicecontrol/src/controlinterface.cpp b/deepin-devicemanager-server/deepin-devicecontrol/src/controlinterface.cpp
index 105f5e60..safe_code_patch 100644
--- a/deepin-devicemanager-server/deepin-devicecontrol/src/controlinterface.cpp
+++ b/deepin-devicemanager-server/deepin-devicecontrol/src/controlinterface.cpp
@@ -333,6 +333,13 @@ bool ControlInterface::authorizedEnable(const QString &hclass, const QString &na
     }
     if (enable_device) {
+        // 安全校验:防止路径遍历和参数注入
+        QRegularExpression validPathRegex("^[a-zA-Z0-9\\-\\./:]+$");
+        if (!validPathRegex.match(path).hasMatch() || path.contains("..")) {
+            qWarning() << "Invalid path detected:" << path;
+            return false;
+        }
+
         /*
          启用流程需等价于物理拔插的「断开—重连」,使 usbhid 重新绑定、
          输入节点重建,否则鼠标启用后仍无响应(PMS 341577)。
@@ -368,8 +375,11 @@ bool ControlInterface::authorizedEnable(const QString &hclass, const QString &na
 
         // 第四步:显式重新授权接口(禁用的直接逆操作)
-        if (file.open(QIODevice::ReadWrite)) {
+        if (!file.open(QIODevice::ReadWrite)) {
+            qWarning() << "Failed to open interface authorized file for re-enabling:" << file.fileName();
+            return false;
+        }
+        {
             file.write("1");
             file.close();
         }
@@ -377,8 +387,9 @@ bool ControlInterface::authorizedEnable(const QString &hclass, const QString &na
         // 第五步:触发 udev 重新处理设备添加事件,确保输入设备节点重建
-        QProcess::execute("udevadm", QStringList() << "trigger"
-                          << "--action=add" << ("--syspath=" + QString("/sys") + pop));
+        QString sysPath = "/sys" + pop;
+        if (sysPath.startsWith("/sys/devices/")) {
+            QProcess::startDetached("udevadm", QStringList() << "trigger" << "--action=add" << ("--syspath=" + sysPath));
+        }
 
         EnableSqlManager::getInstance()->removeDataFromAuthorizedTable(unique_id);

@deepin-ci-robot

Copy link
Copy Markdown

@GongHeng2017: 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 20a9679 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.

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.

2 participants