From 9c73e0dd8e1e57e57b9904098929f7f4133f574a Mon Sep 17 00:00:00 2001 From: rewine Date: Fri, 9 Oct 2026 14:59:51 +0800 Subject: [PATCH] fix(session): store logind session id as string to avoid collision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/treeland#1464 --- src/greeter/greeterproxy.cpp | 17 +++++++++-------- src/greeter/usermodel.h | 2 +- src/seat/helper.cpp | 6 +++--- src/seat/helper.h | 2 +- src/session/session.cpp | 34 +++++++++++++++++++++++----------- src/session/session.h | 15 +++++++++------ 6 files changed, 46 insertions(+), 30 deletions(-) diff --git a/src/greeter/greeterproxy.cpp b/src/greeter/greeterproxy.cpp index 60fa1e87c..22a4b5713 100644 --- a/src/greeter/greeterproxy.cpp +++ b/src/greeter/greeterproxy.cpp @@ -318,8 +318,9 @@ void GreeterProxy::onSessionNew(const QString &id, [[maybe_unused]] const QDBusO QString user = QString::fromLocal8Bit(username); qCInfo(lcTlGreeter) << "New session added: id=" << id << ", user=" << user; userModel()->updateUserLoginState(user, true); + // userLoggedIn signal is connected with Helper::updateActiveUserSession - Q_EMIT userModel()->userLoggedIn(user, id.toInt()); + Q_EMIT userModel()->userLoggedIn(user, id); // Connect to Lock/Unlock signals auto conn = QDBusConnection::systemBus(); @@ -362,7 +363,7 @@ void GreeterProxy::onSessionRemoved(const QString &id, [[maybe_unused]] const QD this, SLOT(onSessionUnlock())); - auto session = Helper::instance()->sessionManager()->sessionForId(id.toInt()); + auto session = Helper::instance()->sessionManager()->sessionForId(id); if (session) { QString username = session->username(); qCInfo(lcTlGreeter) << "Session removed: id=" << id << ", user=" << username; @@ -385,7 +386,7 @@ void GreeterProxy::onSessionLock() OrgFreedesktopLogin1SessionInterface session("org.freedesktop.login1", path, QDBusConnection::systemBus()); - int id = session.id().toInt(); + QString id = session.id(); qCInfo(lcTlGreeter) << "Lock signal received for session id:" << id; auto activeSession = Helper::instance()->sessionManager()->activeSession().lock(); if (!activeSession) @@ -408,7 +409,7 @@ void GreeterProxy::onSessionUnlock() OrgFreedesktopLogin1SessionInterface session("org.freedesktop.login1", path, QDBusConnection::systemBus()); - int id = session.id().toInt(); + QString id = session.id(); const QString username = session.name(); qCInfo(lcTlGreeter) << "Unlock signal received for session id:" << id; auto activeSession = Helper::instance()->sessionManager()->activeSession().lock(); @@ -419,7 +420,7 @@ void GreeterProxy::onSessionUnlock() qCWarning(lcTlGreeter) << "Unlock signal received for non-active session id:" << id << ", lock it back."; QMetaObject::invokeMethod(this, [this, id] { - SocketWriter(m_socket) << quint32(GreeterMessages::Lock) << QString::number(id); + SocketWriter(m_socket) << quint32(GreeterMessages::Lock) << id; }); } else { QMetaObject::invokeMethod(this, [this] { @@ -530,7 +531,7 @@ void GreeterProxy::readyRead() #endif case DaemonMessages::UserActivateMessage: { QString user; - int sessionId; + QString sessionId; input >> user >> sessionId; // NOTE: maybe DDM will active dde user. @@ -563,7 +564,7 @@ void GreeterProxy::readyRead() } break; case DaemonMessages::UserLoggedIn: { QString user; - int sessionId; + QString sessionId; input >> user >> sessionId; // This will happen after a crash recovery of treeland @@ -590,7 +591,7 @@ void GreeterProxy::readyRead() OrgFreedesktopLogin1ManagerInterface manager("org.freedesktop.login1", Logind::managerPath(), conn); - auto reply = manager.GetSession(QString::number(sessionId)); + auto reply = manager.GetSession(sessionId); reply.waitForFinished(); if (!reply.isValid()) { qCWarning(lcTlGreeter) << "Failed to get session path for session id:" << sessionId << ", error:" << reply.error().message(); diff --git a/src/greeter/usermodel.h b/src/greeter/usermodel.h index 1d787983e..194d76799 100644 --- a/src/greeter/usermodel.h +++ b/src/greeter/usermodel.h @@ -86,7 +86,7 @@ class UserModel : public QAbstractListModel void currentUserNameChanged(); void updateTranslations(const QLocale &locale); void countChanged(); - void userLoggedIn(const QString &username, int sessionId); + void userLoggedIn(const QString &username, const QString &sessionId); private Q_SLOTS: void onUserAdded(quint64 uid); diff --git a/src/seat/helper.cpp b/src/seat/helper.cpp index 02d9f5652..605fbf2b6 100644 --- a/src/seat/helper.cpp +++ b/src/seat/helper.cpp @@ -2289,8 +2289,8 @@ void Helper::init(Treeland::Treeland *treeland) this, updateXWaylandOutputScale); updateXWaylandOutputScale(); - // User dde does not has a real Logind session, so just pass 0 as id - m_sessionManager->updateActiveUserSession(QStringLiteral("dde"), 0); + // User dde does not has a real Logind session, so just pass "0" as id + m_sessionManager->updateActiveUserSession(QStringLiteral("dde"), QStringLiteral("0")); connect(m_userModel, &UserModel::userLoggedIn, m_sessionManager, &SessionManager::updateActiveUserSession); m_xdgDecorationManager = m_server->attach(); connect(m_xdgDecorationManager, @@ -3832,7 +3832,7 @@ DDMInterfaceV1 *Helper::ddmInterfaceV1() const { return m_ddmInterfaceV1; } -bool Helper::activateUserSession(const QString &username, int sessionId) +bool Helper::activateUserSession(const QString &username, const QString &sessionId) { if (!m_userModel->getUser(username)) return false; diff --git a/src/seat/helper.h b/src/seat/helper.h index 03a185acb..ee40be5ed 100644 --- a/src/seat/helper.h +++ b/src/seat/helper.h @@ -264,7 +264,7 @@ class Helper : public WSeatEventFilter, public WAYLIB_SERVER_NAMESPACE::WObject inline SessionModel *sessionModel() const { return m_sessionModel; }; DDMInterfaceV1 *ddmInterfaceV1() const; - bool activateUserSession(const QString &username, int sessionId); + bool activateUserSession(const QString &username, const QString &sessionId); void enableRender(); void disableRender(); diff --git a/src/session/session.cpp b/src/session/session.cpp index d0bdb58c7..ac52e295a 100644 --- a/src/session/session.cpp +++ b/src/session/session.cpp @@ -86,7 +86,7 @@ Session::~Session() } } -int Session::id() const +const QString &Session::id() const { return m_id; } @@ -227,10 +227,12 @@ bool SessionManager::activeSocketEnabled() const void SessionManager::setActiveSocketEnabled(bool newEnabled) { auto ptr = m_activeSession.lock(); - if (ptr && ptr->m_socket) - ptr->m_socket->setEnabled(newEnabled, globalSession()->socket()); - else + if (ptr && ptr->m_socket) { + const auto global = globalSession(); + ptr->m_socket->setEnabled(newEnabled, global ? global->socket() : nullptr); + } else { qCWarning(lcTlCore) << "Can't set enabled for empty socket!"; + } } /** @@ -264,7 +266,7 @@ void SessionManager::removeSession(std::shared_ptr session) * @param username Username to ensure session for * @returns Session for the given username, or nullptr on failure */ -std::shared_ptr SessionManager::ensureSession(int id, QString username) +std::shared_ptr SessionManager::ensureSession(const QString &id, QString username) { // Helper lambda to create WSocket and WXWayland auto createWSocket = [this]() { @@ -419,7 +421,7 @@ std::shared_ptr SessionManager::ensureSession(int id, QString username) * @param id Session ID to find session for * @returns Session for the given id, or nullptr if not found */ -std::shared_ptr SessionManager::sessionForId(int id) const +std::shared_ptr SessionManager::sessionForId(const QString &id) const { for (const auto &session : std::as_const(m_sessions)) { if (session && session->m_id == id) @@ -490,7 +492,15 @@ std::shared_ptr SessionManager::sessionForSocket(WSocket *socket) const bool SessionManager::isDDEUserClient(WClient *client) { - return client->socket() == globalSession()->socket(); + // The dde sentinel session is created at startup, but defensive null + // checks guard against it being absent (see issue #1464): dereferencing + // a null globalSession() here previously caused a SIGSEGV. + if (!client) + return false; + const auto global = globalSession(); + if (!global || !global->socket()) + return false; + return client->socket() == global->socket(); } void SessionManager::syncActiveSessionCursorSettings() @@ -526,13 +536,13 @@ void SessionManager::syncActiveSessionCursorSettings() * * @param username Username to set as active session */ -void SessionManager::updateActiveUserSession(const QString &username, int id) +void SessionManager::updateActiveUserSession(const QString &username, const QString &id) { commitActiveUserSession(prepareActiveUserSession(username, id)); } SessionManager::ActiveSessionUpdate SessionManager::prepareActiveUserSession(const QString &username, - int id) + const QString &id) { // Get previous active session auto previous = m_activeSession.lock(); @@ -551,8 +561,10 @@ SessionManager::ActiveSessionUpdate SessionManager::prepareActiveUserSession(con // TODO: Each Wayland socket's active surface needs to be cleaned up individually. Helper::instance()->activateSurface(nullptr); // Update socket enabled state before publishing activation notifications. - if (previous && previous->m_socket) - previous->m_socket->setEnabled(false, globalSession()->socket()); + if (previous && previous->m_socket) { + const auto global = globalSession(); + previous->m_socket->setEnabled(false, global ? global->socket() : nullptr); + } session->m_socket->setEnabled(true); } diff --git a/src/session/session.h b/src/session/session.h index 36a44c01e..4a8c02edc 100644 --- a/src/session/session.h +++ b/src/session/session.h @@ -22,7 +22,7 @@ class Session : public QObject { public: ~Session(); - int id() const; + const QString &id() const; uid_t uid() const; const QString &username() const; WSocket *socket() const; @@ -35,7 +35,10 @@ class Session : public QObject { private: friend class SessionManager; - int m_id = 0; + // The logind session id. Set once in ensureSession() and never mutated + // afterwards; it is read from QThreadPool workers (onSessionLock/Unlock), + // so it must not change after creation. + QString m_id; uid_t m_uid = 0; QString m_username = {}; WSocket *m_socket = nullptr; @@ -73,11 +76,11 @@ class SessionManager : public QObject { } }; - ActiveSessionUpdate prepareActiveUserSession(const QString &username, int id); + ActiveSessionUpdate prepareActiveUserSession(const QString &username, const QString &id); void commitActiveUserSession(const ActiveSessionUpdate &update); - void updateActiveUserSession(const QString &username, int id); + void updateActiveUserSession(const QString &username, const QString &id); void removeSession(std::shared_ptr session); - std::shared_ptr sessionForId(int id) const; + std::shared_ptr sessionForId(const QString &id) const; std::shared_ptr sessionForUid(uid_t uid) const; std::shared_ptr sessionForUser(const QString &username) const; std::shared_ptr sessionForXWayland(WXWayland *xwayland) const; @@ -92,7 +95,7 @@ class SessionManager : public QObject { void xwaylandAuthChanged(); private: - std::shared_ptr ensureSession(int id, QString username); + std::shared_ptr ensureSession(const QString &id, QString username); std::weak_ptr m_activeSession; QList> m_sessions;