Skip to content

fix(service): improve service lifecycle management with D-Bus based detection#207

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/1071from
wangrong1069:pr0616
Jun 17, 2026
Merged

fix(service): improve service lifecycle management with D-Bus based detection#207
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/1071from
wangrong1069:pr0616

Conversation

@wangrong1069

Copy link
Copy Markdown
Contributor

Summary

  • Replace pidof-based process detection with D-Bus GetNameOwner for reliable service startup detection
  • Add authorization check to Quit() method to prevent unauthorized termination
  • Implement graceful service shutdown with PID fallback in service/main.cpp

Related

  • PMS: BUG-364093

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

Please try again later or upgrade to continue using Sourcery

@github-actions

Copy link
Copy Markdown
  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "application/main.cpp": [
        {
            "line": "    const QString acknowledgementLink = \"https://www.deepin.org/acknowledgments/deepin_reader\";",
            "line_number": 80,
            "rule": "S35",
            "reason": "Url link | c08ef69076"
        }
    ]
}

max-lvs
max-lvs previously approved these changes Jun 16, 2026
…etection

Replace pidof-based process detection with D-Bus GetNameOwner for more
reliable service startup detection. Add authorization check to Quit()
method and implement a graceful shutdown by calling the Quit() method.

使用 D-Bus GetNameOwner 替代 pidof 进行服务启动检测,提高可靠性。
为 Quit 方法添加鉴权检查,并通过调用 Quit 方法来实现优雅关闭。

Log: 改进服务生命周期管理,使用D-Bus检测替代pidof
PMS: BUG-364093
Influence: 服务启动检测更可靠,避免竞态问题;Quit 接口增加鉴权防止未授权调用。
@github-actions

Copy link
Copy Markdown
  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "application/main.cpp": [
        {
            "line": "    const QString acknowledgementLink = \"https://www.deepin.org/acknowledgments/deepin_reader\";",
            "line_number": 80,
            "rule": "S35",
            "reason": "Url link | c08ef69076"
        }
    ]
}

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:60分

■ 【总体评价】

代码通过D-Bus原生接口重构了进程检测与退出逻辑,有效消除了原有的TOCTOU竞态条件和权限绕过漏洞,但引入了因鉴权逻辑导致的拒绝服务风险及前端死循环缺陷。
逻辑正确但因安全修复引入新的可用性安全漏洞及前端无限等待问题,受安全上限规则约束扣40分。

■ 【详细分析】

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

application/main.cpp 中的 while(1) 循环缺乏超时退出机制。如果后台服务启动失败、D-Bus异常或卡死,前端进程将永久阻塞在该循环中,无法启动也无法退出。
潜在问题:前端进程永久挂起,用户无法得到任何错误反馈,只能强制杀死进程。
建议:在 while 循环中加入最大等待时间限制(如5秒),超时后执行退出或报错逻辑。

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

移除了冗余且不安全的 executCmd 函数,增加了详尽的注释说明设计意图。但 service/main.cpp 中混用了 C 语言的 usleep 函数,且文件末尾的 exit(0x0002) 缩进被意外修改。
潜在问题:混用 C 标准库休眠函数破坏了 Qt 代码风格一致性;缩进异常影响代码整洁度。
建议:将 usleep 替换为 QThread::msleep 以保持风格统一,修正 exit(0x0002) 的缩进。

  • 3.代码性能(良好)✓

将原本频繁拉起 shell 进程执行 pidof 命令的轮询方式,替换为轻量级的 D-Bus 方法调用,大幅降低了系统调用开销和 CPU 占用。后端通过尝试注册服务名来探测释放状态,频率为 200ms,整体性能损耗在可接受范围内。
建议:后端轮询探测时可考虑直接调用 GetNameOwner 判断是否返回错误,代替频繁的注册与反注册操作,以进一步减轻 D-Bus 守护进程的负担。

  • 4.代码安全(存在 1 个安全漏洞)✕

漏洞对比统计:新增漏洞 1 个,减少漏洞 2 个,持平 0 个
修复了原有的命令注入风险、TOCTOU竞态条件和Quit接口的权限绕过漏洞,但新增了由于鉴权机制不匹配导致的拒绝服务漏洞,攻击面位于服务启动阶段的D-Bus交互。

  • 安全漏洞1(【中危】):拒绝服务 在 service/main.cpp 的 stopExistingService 函数中,由于 diskmanagerservice.cpp 中的 Quit() 接口新增了 checkAuthorization() 鉴权校验,当新启动的后台服务进程尝试通过 D-Bus 调用旧服务的 Quit() 方法时,新进程通常不具备通过 Polkit 鉴权的上下文,导致 Quit() 调用必定失败并返回错误,进而触发 stopExistingService 返回 false,最终导致新服务直接执行 exit(0x0001) 退出,造成磁盘管理器服务无法正常启动的拒绝服务状态。——非常重要

  • 建议:将服务退出逻辑拆分为内部退出和外部退出,stopExistingService 内部可调用一个无需鉴权的私有 D-Bus 方法(如 InternalQuit)来优雅关闭旧服务;或者在前端 application/main.cpp 中负责调用 Quit() 退出旧服务(前端具备用户授权上下文),后端仅做重试注册。

■ 【改进建议代码示例】

// application/main.cpp:增加超时机制防止死循环
// 启动服务后,循环查询后台服务是否已经启动
const int maxWaitMs = 5000;
const int pollIntervalMs = 300;
for (int elapsed = 0; elapsed < maxWaitMs; elapsed += pollIntervalMs) {
    QDBusReply<QString> newReply = dbusInterface.call("GetNameOwner", serviceName);
    if (newReply.isValid()) {
        const QString newDbusOwner = newReply.value();
        if (!newDbusOwner.isEmpty() && newDbusOwner != oldDbusOwner) {
            break;
        }
    }
    QThread::msleep(pollIntervalMs);
}

// service/diskmanagerservice.cpp:拆分退出接口,内部退出无需鉴权
void DiskManagerService::Quit()
{
    if (!checkAuthorization())
        return;

    qDebug() << "DiskManagerService::Quit called by external client";
    gracefulExit();
}

void DiskManagerService::InternalQuit()
{
    qDebug() << "DiskManagerService::InternalQuit called by new instance";
    gracefulExit();
}

void DiskManagerService::gracefulExit()
{
    m_partedcore->delTempMountFile();
    QCoreApplication::exit(0);
}

// service/main.cpp:调用无需鉴权的内部退出接口,并替换 usleep
static bool stopExistingService(QDBusConnection &systemBus)
{
    QDBusInterface existingService(DiskManagerServiceName, DiskManagerPath, DiskManagerServiceName, systemBus);
    if (!existingService.isValid()) {
        qWarning() << "Cannot access existing service via D-Bus interface,"
                   << "service may have already exited, retry registration";
        return true;
    }

    qDebug() << "Calling InternalQuit() on existing service...";
    QDBusPendingCall pendingCall = existingService.asyncCall("InternalQuit");
    pendingCall.waitForFinished();
    if (pendingCall.isError()) {
        qWarning() << "InternalQuit() call failed:" << pendingCall.error().message();
        return false;
    }

    qDebug() << "InternalQuit() called successfully, waiting for service to exit...";
    const int maxWaitMs = 3000;
    const int pollIntervalMs = 200;
    for (int elapsed = 0; elapsed < maxWaitMs; elapsed += pollIntervalMs) {
        QThread::msleep(pollIntervalMs); // 替换 usleep
        if (systemBus.registerService(DiskManagerServiceName)) {
            systemBus.unregisterService(DiskManagerServiceName);
            qDebug() << "Service name released after" << (elapsed + pollIntervalMs) << "ms";
            return true;
        }
    }

    qWarning() << "Timed out waiting for existing service to exit";
    return false;
}

@wangrong1069

Copy link
Copy Markdown
Contributor Author

deepin pr auto review

★ 总体评分:60分

■ 【总体评价】

代码通过D-Bus原生接口重构了进程检测与退出逻辑,有效消除了原有的TOCTOU竞态条件和权限绕过漏洞,但引入了因鉴权逻辑导致的拒绝服务风险及前端死循环缺陷。
逻辑正确但因安全修复引入新的可用性安全漏洞及前端无限等待问题,受安全上限规则约束扣40分。

■ 【详细分析】

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

application/main.cpp 中的 while(1) 循环缺乏超时退出机制。如果后台服务启动失败、D-Bus异常或卡死,前端进程将永久阻塞在该循环中,无法启动也无法退出。
潜在问题:前端进程永久挂起,用户无法得到任何错误反馈,只能强制杀死进程。
建议:在 while 循环中加入最大等待时间限制(如5秒),超时后执行退出或报错逻辑。

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

移除了冗余且不安全的 executCmd 函数,增加了详尽的注释说明设计意图。但 service/main.cpp 中混用了 C 语言的 usleep 函数,且文件末尾的 exit(0x0002) 缩进被意外修改。
潜在问题:混用 C 标准库休眠函数破坏了 Qt 代码风格一致性;缩进异常影响代码整洁度。
建议:将 usleep 替换为 QThread::msleep 以保持风格统一,修正 exit(0x0002) 的缩进。

  • 3.代码性能(良好)✓

将原本频繁拉起 shell 进程执行 pidof 命令的轮询方式,替换为轻量级的 D-Bus 方法调用,大幅降低了系统调用开销和 CPU 占用。后端通过尝试注册服务名来探测释放状态,频率为 200ms,整体性能损耗在可接受范围内。
建议:后端轮询探测时可考虑直接调用 GetNameOwner 判断是否返回错误,代替频繁的注册与反注册操作,以进一步减轻 D-Bus 守护进程的负担。

  • 4.代码安全(存在 1 个安全漏洞)✕

漏洞对比统计:新增漏洞 1 个,减少漏洞 2 个,持平 0 个
修复了原有的命令注入风险、TOCTOU竞态条件和Quit接口的权限绕过漏洞,但新增了由于鉴权机制不匹配导致的拒绝服务漏洞,攻击面位于服务启动阶段的D-Bus交互。

  • 安全漏洞1(【中危】):拒绝服务 在 service/main.cpp 的 stopExistingService 函数中,由于 diskmanagerservice.cpp 中的 Quit() 接口新增了 checkAuthorization() 鉴权校验,当新启动的后台服务进程尝试通过 D-Bus 调用旧服务的 Quit() 方法时,新进程通常不具备通过 Polkit 鉴权的上下文,导致 Quit() 调用必定失败并返回错误,进而触发 stopExistingService 返回 false,最终导致新服务直接执行 exit(0x0001) 退出,造成磁盘管理器服务无法正常启动的拒绝服务状态。——非常重要
  • 建议:将服务退出逻辑拆分为内部退出和外部退出,stopExistingService 内部可调用一个无需鉴权的私有 D-Bus 方法(如 InternalQuit)来优雅关闭旧服务;或者在前端 application/main.cpp 中负责调用 Quit() 退出旧服务(前端具备用户授权上下文),后端仅做重试注册。

■ 【改进建议代码示例】

// application/main.cpp:增加超时机制防止死循环
// 启动服务后,循环查询后台服务是否已经启动
const int maxWaitMs = 5000;
const int pollIntervalMs = 300;
for (int elapsed = 0; elapsed < maxWaitMs; elapsed += pollIntervalMs) {
    QDBusReply<QString> newReply = dbusInterface.call("GetNameOwner", serviceName);
    if (newReply.isValid()) {
        const QString newDbusOwner = newReply.value();
        if (!newDbusOwner.isEmpty() && newDbusOwner != oldDbusOwner) {
            break;
        }
    }
    QThread::msleep(pollIntervalMs);
}

// service/diskmanagerservice.cpp:拆分退出接口,内部退出无需鉴权
void DiskManagerService::Quit()
{
    if (!checkAuthorization())
        return;

    qDebug() << "DiskManagerService::Quit called by external client";
    gracefulExit();
}

void DiskManagerService::InternalQuit()
{
    qDebug() << "DiskManagerService::InternalQuit called by new instance";
    gracefulExit();
}

void DiskManagerService::gracefulExit()
{
    m_partedcore->delTempMountFile();
    QCoreApplication::exit(0);
}

// service/main.cpp:调用无需鉴权的内部退出接口,并替换 usleep
static bool stopExistingService(QDBusConnection &systemBus)
{
    QDBusInterface existingService(DiskManagerServiceName, DiskManagerPath, DiskManagerServiceName, systemBus);
    if (!existingService.isValid()) {
        qWarning() << "Cannot access existing service via D-Bus interface,"
                   << "service may have already exited, retry registration";
        return true;
    }

    qDebug() << "Calling InternalQuit() on existing service...";
    QDBusPendingCall pendingCall = existingService.asyncCall("InternalQuit");
    pendingCall.waitForFinished();
    if (pendingCall.isError()) {
        qWarning() << "InternalQuit() call failed:" << pendingCall.error().message();
        return false;
    }

    qDebug() << "InternalQuit() called successfully, waiting for service to exit...";
    const int maxWaitMs = 3000;
    const int pollIntervalMs = 200;
    for (int elapsed = 0; elapsed < maxWaitMs; elapsed += pollIntervalMs) {
        QThread::msleep(pollIntervalMs); // 替换 usleep
        if (systemBus.registerService(DiskManagerServiceName)) {
            systemBus.unregisterService(DiskManagerServiceName);
            qDebug() << "Service name released after" << (elapsed + pollIntervalMs) << "ms";
            return true;
        }
    }

    qWarning() << "Timed out waiting for existing service to exit";
    return false;
}

1.语法逻辑(存在错误)
不存在该问题, 前端进程没有显示出来就是错误反馈

4.代码安全(存在 1 个安全漏洞)
不存在该问题, 新服务以 root 账户运行, 能够直接通过 checkAuthorization() 鉴权校验

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: max-lvs, 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

@wangrong1069

Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot Bot merged commit 0f61af8 into linuxdeepin:release/1071 Jun 17, 2026
22 checks passed
@wangrong1069 wangrong1069 deleted the pr0616 branch June 17, 2026 02:23
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