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: