Repository navigation
fix(session): store logind session id as string to avoid collision - #1465
Conversation
Reviewer's GuideMigrates logind session IDs from integers to strings so IDs such as “c1” cannot collide with the DDE sentinel “0”, while preserving protocol compatibility at existing boundaries and adding defensive checks around absent global/session sockets. Sequence diagram for string-based logind session cleanupsequenceDiagram
participant Logind
participant GreeterProxy
participant SessionManager
participant Session
Logind->>GreeterProxy: onSessionRemoved(id)
GreeterProxy->>SessionManager: sessionForId(id)
SessionManager->>SessionManager: Compare raw QString id
SessionManager-->>GreeterProxy: Matching Session or nullptr
GreeterProxy->>Session: username()
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
崩溃修复报告：logind 会话移除时误删 dde 哨兵会话摘要treeland 在 DDM 锁屏模式下（ 根因是 影响版本
崩溃现场关键代码： // src/session/session.cpp:493
bool SessionManager::isDDEUserClient(WClient *client)
{
return client->socket() == globalSession()->socket();
}
根因分析1. dde 哨兵会话使用 id=0启动时（ // User dde does not has a real Logind session, so just pass 0 as id
m_sessionManager->updateActiveUserSession(QStringLiteral("dde"), 0);dde 是一个不对应真实 logind 会话的哨兵会话，以 2. logind 无 seat 会话的 id 是非数字字符串systemd-logind 的会话 id 命名规则：
journal 中的证据：
3.
|
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved blocking issues were identified.
0 open findings
What changed in this PR
Fixes logind session removal by ignoring non-numeric IDs such as c1, preventing accidental removal of the DDE sentinel session.
Changes:
- Validate session IDs before conversion.
- Ignore invalid IDs with debug logging.
- Use validated IDs for session lookup.
| File | Description |
|---|---|
src/greeter/greeterproxy.cpp |
Safely validates and handles removed logind session IDs. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
研究完成。基于 systemd 源码，我可以给你完整的、有源码依据的结论了。 一、c1/c2 的确切生成规则（源码证据） src/login/logind-dbus.c → manager_choose_session_id()（约第 838-880 行）： /* Try to keep our session IDs and the audit session IDs in sync */
uint32_t audit_id = AUDIT_SESSION_INVALID;
r = audit_session_from_pid(leader, &audit_id);
if (r < 0) {
// 读 audit id 失败 → 进入下面 fallback
} else {
asprintf(&id, "%"PRIu32, audit_id); // 数字 id
...
}
if (!id)
do {
id = mfree(id);
if (asprintf(&id, "c%" PRIu64, ++m->session_counter) < 0) // ← c 前缀来源
return -ENOMEM;
} while (hashmap_contains(m->sessions, id)); 结论：
src/basic/audit-util.c → audit_session_from_pid()： r = audit_read_field(pid, "sessionid", &s); // 读 /proc/<pid>/sessionid
...
if (!audit_session_is_valid(u)) // id > 0 && id != UINT32_MAX
return -ENODATA; 读的是 /proc/<leader_pid>/sessionid，这个文件由内核 audit 子系统（pam_loginuid.so 设置 loginuid 后）提 而 DDM 的图形登录走完整 PAM（pam_loginuid.so），leader 进程有有效 audit session id（如 1），所以 id 是 二、为什么 onSessionRemoved 拿不到 service（源码证据） src/login/logind-session.c → session_finalize()（约第 1043 行起）： int session_finalize(Session *s) {
...
if (s->started)
log_struct(..., LOG_MESSAGE("Removed session %s.", s->id)); // ← 先打 "Removed session c1."
...
if (s->started) {
session_send_signal(s, false); // ← 然后发 SessionRemoved 信号
s->started = false;
}
...
session_add_to_gc_queue(s); // ← 加入 GC 队列
...
} session_send_signal()（logind-session-dbus.c:839）： sd_bus_emit_signal(..., new_session ? "SessionNew" : "SessionRemoved", "so", s->id, p); 随后 GC 队列处理（logind.c:1095）会调用 session_free()，其中： free(s->service); // ← 释放 service 字符串
hashmap_remove(s->manager->sessions, s->id); // ← 从 hashmap 移除
free(s->id); 关键时序：
我用实际程序验证过（查一个不存在的会话 c999）：sd_session_get_service 返回 -6（ENXIO）。 对比 SessionNew：session_start() 里是先创建好会话对象、填充 service、加入 hashmap，然后才发 SessionNew 三、结论（回答你最初的问题） ┌────────────────────────────┬──────────────────┬─────────────────────────────┐ 所以 onSessionRemoved 不是"忘了判断 service"，而是 systemd 的 SessionRemoved 信号在会话销毁后才发出，回 四、对 onSessionNew 是否加 toInt 检查的最终建议 基于源码，现在可以更准确地判断：
我的建议：不需要改 onSessionNew。 它是安全的，因为：
当前修复（只改 onSessionRemoved）已经完整覆盖了根因。 |
f5cd407 to
b3e88ab
Compare
wineee
changed the title
1. Migrate Session::m_id and the SessionManager session-id APIs from int
to QString
2. Remove the toInt() validation in onSessionRemoved; look up the raw id
string so "c1" can no longer collide with the dde sentinel session ("0")
3. Keep the DDM socket protocol as int for now, converting at the boundary
Log: No user-facing changes
Influence:
1. Reproduce the crash under a DDM-managed treeland (--lockscreen)
2. Run a root su/runuser to deepin and confirm the logind session ("c1")
no longer removes the dde sentinel session
3. Verify the wallpaper shell protocol still works for the dde user client
fix(session): 用字符串存储 logind 会话 id 避免碰撞
1. 将 Session::m_id 及 SessionManager 会话 id 接口从 int 迁移为 QString
2. 移除 onSessionRemoved 中的 toInt 校验，直接按原始字符串查找，
使 "c1" 不再与 dde 哨兵会话("0")碰撞
3. DDM socket 协议暂保持 int，在边界处转换
Log: 无用户可见变化
Influence:
1. 在 DDM 管理的 treeland(--lockscreen)下复现崩溃
2. 执行 root 的 su/runuser 切换到 deepin，确认 "c1" 会话不再误删 dde 哨兵会话
3. 验证 dde 用户客户端的 wallpaper shell 协议仍正常
Fixes: linuxdeepin#1464
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/greeter/greeterproxy.cpp" line_range="534" />
<code_context>
case DaemonMessages::UserActivateMessage: {
QString user;
- int sessionId;
+ QString sessionId;
input >> user >> sessionId;
</code_context>
<issue_to_address>
**Session IDs break DDM messages**
When DDM exchanges session-ID fields using its existing integer socket protocol, `readyRead()` reads activation and login IDs as `QString`, while the `Logout` and `Lock` writers serialize `Session::id()` as a string. The mismatched `QDataStream` types make DDM session messages fail to parse, so activation, recovery, locking, or logout fails.
Keep session IDs as integers at DDM socket boundaries, converting to or from `QString` only for internal session handling.
Also at `src/greeter/greeterproxy.cpp:422-423`, `src/greeter/greeterproxy.cpp:567-568`, `src/session/session.h:25`, `src/session/session.h:41`, `src/session/session.cpp:89`.
</issue_to_address>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wineee, zccrs 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 |
to QString
string so "c1" can no longer collide with the dde sentinel session ("0")
Log: No user-facing changes
Influence:
no longer removes the dde sentinel session
fix(session): 用字符串存储 logind 会话 id 避免碰撞
使 "c1" 不再与 dde 哨兵会话("0")碰撞
Log: 无用户可见变化
Influence:
Fixes: #1464
Summary by Sourcery
Preserve raw logind session identifiers to prevent session collisions and improve handling of missing global sessions.
Bug Fixes:
Enhancements: