From 89e59b7209e6aff862fc24fdaecf44574465b39d Mon Sep 17 00:00:00 2001 From: rewine Date: Fri, 9 Oct 2026 18:21:40 +0800 Subject: [PATCH] feat: store logind session id as string instead of int MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Change Auth::xdgSessionId and openSession() to QString so non-numeric logind session ids (e.g. "c1") are preserved end to end. 2. Pass the session id over the parent/child pipe as length-prefixed bytes instead of a raw int. 3. Update SocketServer, Display and TreelandDisplayServer to send and receive session ids as strings. Log: No user-facing changes Influence: 1. Verify a normal user session still activates, locks, unlocks and logs out correctly after login via ddm. 2. Verify switching back to the greeter (dde session with empty id) does not crash or misroute the session. 3. Verify non-numeric logind session ids no longer collide with the dde greeter sentinel session. feat: 将 logind 会话 id 由 int 改为 string 存储 1. 将 Auth::xdgSessionId 与 openSession() 改为 QString，使 c1 这类非 数字会话 id 可端到端透传。 2. 父子进程间改为以「长度+字节」方式传递会话 id，取代原始 int。 3. 更新 SocketServer、Display 与 TreelandDisplayServer，以字符串收 发会话 id。 Log: 无用户可见变化 Influence: 1. 验证通过 ddm 登录后，普通用户会话的激活、锁定、解锁、注销均正常。 2. 验证切回登录界面（dde 会话 id 为空）不会崩溃或误路由会话。 3. 验证非数字 logind 会话 id 不再与 dde 登录界面哨兵会话冲突。 Fixes: linuxdeepin/treeland#1464 --- src/daemon/Auth.cpp | 112 +++++++++++++++++++++++---- src/daemon/Auth.h | 12 +-- src/daemon/Display.cpp | 26 +++---- src/daemon/Display.h | 14 ++-- src/daemon/SeatManager.cpp | 13 +++- src/daemon/SocketServer.cpp | 84 +++++++++++++++----- src/daemon/SocketServer.h | 4 +- src/daemon/TreelandDisplayServer.cpp | 10 +-- src/daemon/TreelandDisplayServer.h | 2 +- 9 files changed, 206 insertions(+), 71 deletions(-) diff --git a/src/daemon/Auth.cpp b/src/daemon/Auth.cpp index ee467a4..76e9e87 100644 --- a/src/daemon/Auth.cpp +++ b/src/daemon/Auth.cpp @@ -64,6 +64,59 @@ namespace DDM { } } + // Complete-transfer helpers for the parent/child session pipe. + // Plain read()/write() may return fewer bytes than requested (short + // transfer) or fail with EINTR when interrupted by a signal. Treating + // those as fatal would leak an already-opened logind session, so retry + // until the whole frame is transferred or a real error/EOF occurs. + static bool writeAll(int fd, const void *buffer, size_t size) + { + const auto *data = static_cast(buffer); + size_t written = 0; + while (written < size) { + const ssize_t ret = write(fd, data + written, size - written); + if (ret == -1) { + if (errno == EINTR) + continue; + return false; + } + if (ret == 0) { + // write() returning 0 means nothing was transferred. On a + // blocking pipe this should not happen for a non-zero size, + // but treat it as a hard error to avoid an infinite loop. + errno = EIO; + return false; + } + written += static_cast(ret); + } + return true; + } + + static bool readFull(int fd, void *buffer, size_t size) + { + auto *data = static_cast(buffer); + size_t received = 0; + while (received < size) { + const ssize_t ret = read(fd, data + received, size - received); + if (ret == -1) { + if (errno == EINTR) + continue; + return false; + } + if (ret == 0) { // EOF: the child closed the pipe early + errno = EPIPE; + return false; + } + received += static_cast(ret); + } + return true; + } + + // XDG_SESSION_ID is a short opaque string (numeric or e.g. "c1"). The + // length prefix is validated against this bound to avoid allocating a + // huge buffer from a corrupted/oversized pipe frame. + constexpr quint32 kMaxSessionIdLen = 256; + /////////////////////////// // utmp helper functions // /////////////////////////// @@ -262,15 +315,15 @@ namespace DDM { return true; } - int Auth::openSession(const QString &command, - QProcessEnvironment env, - const QByteArray &cookie) { + QString Auth::openSession(const QString &command, + QProcessEnvironment env, + const QByteArray &cookie) { Q_ASSERT(authenticated); int pipefd[2]; if (pipe(pipefd) == -1) { qWarning() << "[Auth] pipe failed:" << strerror(errno); - return -1; + return {}; } // Here is most safe place to request the VT switch before opening the session. @@ -278,7 +331,7 @@ namespace DDM { qWarning() << "[Auth] Failed to switch to VT" << tty << ":" << strerror(errno); close(pipefd[0]); close(pipefd[1]); - return -1; + return {}; } sessionLeaderPid = fork(); @@ -288,7 +341,7 @@ namespace DDM { qWarning() << "[Auth] fork failed:" << strerror(errno); close(pipefd[0]); close(pipefd[1]); - return -1; + return {}; } case 0: { // Child (session leader) process @@ -328,12 +381,15 @@ namespace DDM { env = *sessionEnv; // Retrieve XDG_SESSION_ID - xdgSessionId = env.value(QStringLiteral("XDG_SESSION_ID")).toInt(); - if (xdgSessionId <= 0) { + xdgSessionId = env.value(QStringLiteral("XDG_SESSION_ID")); + if (xdgSessionId.isEmpty()) { qCritical() << "[SessionLeader] Invalid XDG_SESSION_ID from pam_open_session()"; exit(1); } - if (write(pipefd[1], &xdgSessionId, sizeof(int)) != sizeof(int)) { + const QByteArray idBytes = xdgSessionId.toLocal8Bit(); + const quint32 idLen = static_cast(idBytes.size()); + if (!writeAll(pipefd[1], &idLen, sizeof(idLen)) + || !writeAll(pipefd[1], idBytes.constData(), idBytes.size())) { qCritical() << "[SessionLeader] Failed to write XDG_SESSION_ID to parent process!"; exit(1); } @@ -349,7 +405,7 @@ namespace DDM { // Send session PID to parent sessionPid = session.processId(); - if (write(pipefd[1], &sessionPid, sizeof(qint64)) != sizeof(qint64)) { + if (!writeAll(pipefd[1], &sessionPid, sizeof(qint64))) { qCritical() << "[SessionLeader] Failed to write session PID to parent process!"; exit(1); } @@ -370,22 +426,44 @@ namespace DDM { // Parent process close(pipefd[1]); - if (read(pipefd[0], &xdgSessionId, sizeof(int)) < 0) { + quint32 idLen = 0; + if (!readFull(pipefd[0], &idLen, sizeof(idLen))) { + qWarning() << "[Auth] Failed to read XDG_SESSION_ID from child process:" << strerror(errno); + close(pipefd[0]); + return {}; + } + // XDG_SESSION_ID is a short opaque string (numeric or e.g. "c1"), + // so guard against a corrupted/oversized length field from the pipe. + if (idLen == 0 || idLen > kMaxSessionIdLen) { + qWarning() << "[Auth] Invalid XDG_SESSION_ID length:" << idLen; + close(pipefd[0]); + return {}; + } + QByteArray idBytes(static_cast(idLen), Qt::Uninitialized); + if (!readFull(pipefd[0], idBytes.data(), idLen)) { qWarning() << "[Auth] Failed to read XDG_SESSION_ID from child process:" << strerror(errno); close(pipefd[0]); - return -1; + return {}; } - if (read(pipefd[0], &sessionPid, sizeof(qint64)) < 0) { + xdgSessionId = QString::fromLocal8Bit(idBytes); + + if (!readFull(pipefd[0], &sessionPid, sizeof(qint64))) { qWarning() << "[Auth] Failed to read session PID from child process:" << strerror(errno); close(pipefd[0]); - return -1; + return {}; + } + if (sessionPid <= 0) { + qWarning() << "[Auth] Invalid session PID from child process:" << sessionPid; + close(pipefd[0]); + return {}; } utmpLogin(true); // Monitor child process ends - m_notifier = new QSocketNotifier(pipefd[0], QSocketNotifier::Read); - connect(m_notifier, &QSocketNotifier::activated, this, [this, pipefd] { - close(pipefd[0]); + const int readFd = pipefd[0]; + m_notifier = new QSocketNotifier(readFd, QSocketNotifier::Read); + connect(m_notifier, &QSocketNotifier::activated, this, [this, readFd] { + close(readFd); m_notifier->setEnabled(false); m_notifier->deleteLater(); Q_EMIT sessionFinished(); diff --git a/src/daemon/Auth.h b/src/daemon/Auth.h index f4965dc..dc90ce4 100644 --- a/src/daemon/Auth.h +++ b/src/daemon/Auth.h @@ -46,8 +46,8 @@ namespace DDM { /** Virtual terminal number (e.g. 7 for tty7) */ int tty{ 0 }; - /** Logind session ID (the XDG_SESSION_ID env var). Positive values are valid */ - int xdgSessionId{ 0 }; + /** Logind session ID (the XDG_SESSION_ID env var). Empty means no session */ + QString xdgSessionId{}; /** PID of the session leader. Positive values are valid */ pid_t sessionLeaderPid{ 0 }; @@ -71,11 +71,11 @@ namespace DDM { * @param command Command to execute as user process * @param env Environment variables to set for the session * @param cookie XAuth cookie, must be provided if type=X11 - * @return A valid XDG_SESSION_ID on success, zero or negative on failure + * @return A valid XDG_SESSION_ID on success, empty string on failure */ - int openSession(const QString &command, - QProcessEnvironment env, - const QByteArray &cookie = QByteArray()); + QString openSession(const QString &command, + QProcessEnvironment env, + const QByteArray &cookie = QByteArray()); /** * Close PAM session diff --git a/src/daemon/Display.cpp b/src/daemon/Display.cpp index ddea869..c41f4e2 100644 --- a/src/daemon/Display.cpp +++ b/src/daemon/Display.cpp @@ -158,20 +158,20 @@ namespace DDM { stop(); } - void Display::activateSession(const QString &user, int xdgSessionId) { + void Display::activateSession(const QString &user, const QString &xdgSessionId) { qWarning() << "Display activateSession requested for user" << user << "xdgSessionId" << xdgSessionId << "display VT" << terminalId; - if (xdgSessionId <= 0 && user != QStringLiteral("dde")) { + if (xdgSessionId.isEmpty() && user != QStringLiteral("dde")) { qCritical() << "Invalid xdg session id" << xdgSessionId << "for user" << user; return; } m_treeland->activateUser(user, xdgSessionId); - if (xdgSessionId > 0 && Logind::isAvailable()) { + if (!xdgSessionId.isEmpty() && Logind::isAvailable()) { OrgFreedesktopLogin1ManagerInterface manager(Logind::serviceName(), Logind::managerPath(), QDBusConnection::systemBus()); - manager.ActivateSession(QString::number(xdgSessionId)); + manager.ActivateSession(xdgSessionId); } } @@ -418,9 +418,9 @@ namespace DDM { } // Open Logind session & Exec the desktop process - int xdgSessionId = auth->openSession(session.exec(), env, cookie); + const QString xdgSessionId = auth->openSession(session.exec(), env, cookie); - if (xdgSessionId <= 0) { + if (xdgSessionId.isEmpty()) { qCritical() << "Failed to open logind session for user" << auth->user; if (auth->type == Treeland) daemonApp->seatdControl()->destroyGroupVt(auth->tty); @@ -441,7 +441,7 @@ namespace DDM { daemonApp->displayManager()->setLastSession(sessionId); if (auth->type == Treeland) - activateSession(auth->user, xdgSessionId); + activateSession(auth->user, auth->xdgSessionId); qInfo() << "Successfully logged in user" << auth->user; return true; } @@ -507,7 +507,7 @@ namespace DDM { return true; } - void Display::logout([[maybe_unused]] QLocalSocket *socket, int id) { + void Display::logout([[maybe_unused]] QLocalSocket *socket, const QString &id) { qDebug() << "Logout requested for session id" << id; // Do not kill the session leader process before // TerminateSession! Logind will only kill the session's @@ -519,16 +519,16 @@ namespace DDM { OrgFreedesktopLogin1ManagerInterface manager(Logind::serviceName(), Logind::managerPath(), QDBusConnection::systemBus()); - manager.TerminateSession(QString::number(id)); + manager.TerminateSession(id); } - void Display::lock([[maybe_unused]] QLocalSocket *socket, int id) { + void Display::lock([[maybe_unused]] QLocalSocket *socket, const QString &id) { qDebug() << "Lock requested for session id" << id; OrgFreedesktopLogin1ManagerInterface manager(Logind::serviceName(), Logind::managerPath(), QDBusConnection::systemBus()); - manager.LockSession(QString::number(id)); + manager.LockSession(id); } void Display::unlock(QLocalSocket *socket, const QString &user, const QString &password) { @@ -559,11 +559,11 @@ namespace DDM { // Find the auth that started the session, which contains full informations for (auto *auth : std::as_const(auths)) { - if (auth->user == user && auth->xdgSessionId > 0) { + if (auth->user == user && !auth->xdgSessionId.isEmpty()) { OrgFreedesktopLogin1ManagerInterface manager(Logind::serviceName(), Logind::managerPath(), QDBusConnection::systemBus()); - manager.UnlockSession(QString::number(auth->xdgSessionId)); + manager.UnlockSession(auth->xdgSessionId); if (auth->type == Treeland) activateSession(user, auth->xdgSessionId); else if (!daemonApp->seatdControl()->requestSwitchVt(auth->tty)) diff --git a/src/daemon/Display.h b/src/daemon/Display.h index 4bcd19f..496c984 100644 --- a/src/daemon/Display.h +++ b/src/daemon/Display.h @@ -63,13 +63,17 @@ namespace DDM { /** * Tell Treeland to activate a certain session. * - * Called with user = "dde" and xdgSessionId <= 0 - * will send Treeland into lockscreen. + * Called with user = "dde" and an empty xdgSessionId + * will send Treeland into lockscreen. Note: treeland internally + * represents the dde greeter sentinel with id "0" (see treeland + * src/seat/helper.cpp); ddm uses an empty string for the same + * sentinel. The asymmetry is safe because treeland resolves the + * dde session by username, not by this id. * * @param user Username * @param xdgSessionId Logind session ID */ - void activateSession(const QString &user, int xdgSessionId); + void activateSession(const QString &user, const QString &xdgSessionId); /** Seat name */ QString name{}; @@ -104,8 +108,8 @@ namespace DDM { const QString &user, const QString &password, const Session &session); - void logout(QLocalSocket *socket, int id); - void lock(QLocalSocket *socket, int id); + void logout(QLocalSocket *socket, const QString &id); + void lock(QLocalSocket *socket, const QString &id); void unlock(QLocalSocket *socket, const QString &user, const QString &password); signals: diff --git a/src/daemon/SeatManager.cpp b/src/daemon/SeatManager.cpp index 7f90d15..cefef96 100644 --- a/src/daemon/SeatManager.cpp +++ b/src/daemon/SeatManager.cpp @@ -212,8 +212,17 @@ namespace DDM { void SeatManager::switchToGreeter(const QString &name) { for (auto display : std::as_const(displays)) { if (display->name == name) { - // switch to greeter - display->activateSession("dde", 0); + // Switch to greeter. The "dde" user is the greeter sentinel + // with no real logind session; here we pass an empty session id. + // + // NOTE: treeland's startup sentinel uses "0" as the dde session + // id (see treeland src/seat/helper.cpp), while ddm uses an empty + // string. This asymmetry is harmless because treeland resolves + // the dde session by username (sessionForUser("dde")), so the + // empty id never overwrites the already-created "0" sentinel. + // Keep the empty-string sentinel here to avoid coupling ddm's + // wire format to treeland's internal sentinel value. + display->activateSession("dde", QString()); return; } } diff --git a/src/daemon/SocketServer.cpp b/src/daemon/SocketServer.cpp index a040b00..db5e5f9 100644 --- a/src/daemon/SocketServer.cpp +++ b/src/daemon/SocketServer.cpp @@ -113,21 +113,29 @@ namespace DDM { // input stream QDataStream input(socket); - // Qt's QLocalSocket::readyRead is not designed to be called at every socket.write(), - // so we need to use a loop to read all the signals. + // QLocalSocket is stream-oriented: a message header may arrive before + // the complete variable-length payload (e.g. a QString). Read each + // message inside a QDataStream transaction and only act after the + // whole payload has been committed; otherwise the stream is rolled + // back and we wait for more data, avoiding emitting with incomplete + // values and desynchronizing the stream. while(socket->bytesAvailable()) { + input.startTransaction(); + // read message - quint32 message; + quint32 message = 0; input >> message; switch (GreeterMessages(message)) { case GreeterMessages::Connect: { - // log message - qDebug() << "Message received from greeter: Connect"; - // Connect wayland socket QString socketPath; input >> socketPath; + if (!input.commitTransaction()) + return; + + // log message + qDebug() << "Message received from greeter: Connect"; daemonApp->treelandConnector()->connect(socketPath); // send capabilities @@ -141,48 +149,63 @@ namespace DDM { } break; case GreeterMessages::Login: { - // log message - qDebug() << "Message received from greeter: Login"; - // read username, pasword etc. - QString user, password, filename; + QString user, password; Session session; input >> user >> password >> session; + if (!input.commitTransaction()) + return; + + // log message + qDebug() << "Message received from greeter: Login"; // emit signal emit login(socket, user, password, session); } break; case GreeterMessages::Logout: { + // read session id + QString id; + input >> id; + if (!input.commitTransaction()) + return; + // log message qDebug() << "Message received from greeter: Logout"; - // read username - int id; - input >> id; + // emit signal emit logout(socket, id); } break; case GreeterMessages::Lock : { + QString id; + input >> id; + if (!input.commitTransaction()) + return; + // log message qDebug() << "Message received from greeter: Lock"; - int id; - input >> id; emit lock(socket, id); } break; case GreeterMessages::Unlock : { - // log message - qDebug() << "Message received from greeter: Unlock"; QString user; QString password; - input >> user >> password; + if (!input.commitTransaction()) + return; + + // log message + qDebug() << "Message received from greeter: Unlock"; + emit unlock(socket, user, password); } break; case GreeterMessages::PowerOff: { + if (!input.commitTransaction()) + return; + // log message qDebug() << "Message received from greeter: PowerOff"; @@ -191,6 +214,9 @@ namespace DDM { } break; case GreeterMessages::Reboot: { + if (!input.commitTransaction()) + return; + // log message qDebug() << "Message received from greeter: Reboot"; @@ -199,6 +225,9 @@ namespace DDM { } break; case GreeterMessages::Suspend: { + if (!input.commitTransaction()) + return; + // log message qDebug() << "Message received from greeter: Suspend"; @@ -207,6 +236,9 @@ namespace DDM { } break; case GreeterMessages::Hibernate: { + if (!input.commitTransaction()) + return; + // log message qDebug() << "Message received from greeter: Hibernate"; @@ -215,22 +247,34 @@ namespace DDM { } break; case GreeterMessages::HybridSleep: { + if (!input.commitTransaction()) + return; + // log message qDebug() << "Message received from greeter: HybridSleep"; + // hybrid sleep daemonApp->powerManager()->hybridSleep(); } break; case GreeterMessages::BackToNormal: { + if (!input.commitTransaction()) + return; + // log message qDebug() << "Message received from greeter: Back to normal"; - // hybrid sleep + + // back to normal daemonApp->backToNormal(); } break; default: { - // log message + // Unknown message type: its payload length is unknown, so + // it cannot be framed safely. Consume the header and stop + // to avoid treating trailing bytes as a new message header. + input.commitTransaction(); qWarning() << "Unknown message" << message; + return; } } } diff --git a/src/daemon/SocketServer.h b/src/daemon/SocketServer.h index e66382d..fbbbc3d 100644 --- a/src/daemon/SocketServer.h +++ b/src/daemon/SocketServer.h @@ -54,9 +54,9 @@ namespace DDM { const QString &user, const QString &password, const Session &session); void logout(QLocalSocket *socket, - int id); + const QString &id); void lock(QLocalSocket *socket, - int id); + const QString &id); void unlock(QLocalSocket *socket, const QString &user, const QString &password); void connected(QLocalSocket *socket); diff --git a/src/daemon/TreelandDisplayServer.cpp b/src/daemon/TreelandDisplayServer.cpp index 6d1c5bd..86a8147 100644 --- a/src/daemon/TreelandDisplayServer.cpp +++ b/src/daemon/TreelandDisplayServer.cpp @@ -75,17 +75,17 @@ void TreelandDisplayServer::stop() { m_started = false; } -void TreelandDisplayServer::activateUser(const QString &user, int xdgSessionId) { - qDebug("Send greeter activation: user=%s xdgSessionId=%d sockets=%lld", - qPrintable(user), xdgSessionId, static_cast(m_greeterSockets.size())); +void TreelandDisplayServer::activateUser(const QString &user, const QString &xdgSessionId) { + qDebug("Send greeter activation: user=%s xdgSessionId=%s sockets=%lld", + qPrintable(user), qPrintable(xdgSessionId), static_cast(m_greeterSockets.size())); for (auto greeter : m_greeterSockets) { if (user == "dde") { qDebug("Sending SwitchToGreeter to socket=%p", greeter); SocketWriter(greeter) << quint32(DaemonMessages::SwitchToGreeter); } - qDebug("Sending UserActivateMessage to socket=%p user=%s xdgSessionId=%d", - greeter, qPrintable(user), xdgSessionId); + qDebug("Sending UserActivateMessage to socket=%p user=%s xdgSessionId=%s", + greeter, qPrintable(user), qPrintable(xdgSessionId)); SocketWriter(greeter) << quint32(DaemonMessages::UserActivateMessage) << user << xdgSessionId; } } diff --git a/src/daemon/TreelandDisplayServer.h b/src/daemon/TreelandDisplayServer.h index e51eb96..fe156bf 100644 --- a/src/daemon/TreelandDisplayServer.h +++ b/src/daemon/TreelandDisplayServer.h @@ -21,7 +21,7 @@ namespace DDM { public Q_SLOTS: bool start(); void stop(); - void activateUser(const QString &user, int xdgSessionId); + void activateUser(const QString &user, const QString &xdgSessionId); void onLoginFailed(const QString &user); private: