Skip to content

feat: store logind session id as string instead of int #114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zccrs merged 1 commit into from
Oct 10, 2026
Merged

feat: store logind session id as string instead of int #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 95 additions & 17 deletions src/daemon/Auth.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<const char *>(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<size_t>(ret);
}
return true;
}

static bool readFull(int fd, void *buffer, size_t size)
{
auto *data = static_cast<char *>(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<size_t>(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 //
///////////////////////////
Expand Down Expand Up @@ -262,23 +315,23 @@ 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.
if (!daemonApp->seatdControl()->requestSwitchVt(tty)) {
qWarning() << "[Auth] Failed to switch to VT" << tty << ":" << strerror(errno);
close(pipefd[0]);
close(pipefd[1]);
return -1;
return {};
}

sessionLeaderPid = fork();
Expand All @@ -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
Expand Down Expand Up @@ -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<quint32>(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);
}
Expand All @@ -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);
}
Expand All @@ -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 {};
Comment thread
sourcery-ai[bot] marked this conversation as resolved.
}
QByteArray idBytes(static_cast<int>(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();
Expand Down
12 changes: 6 additions & 6 deletions src/daemon/Auth.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand All @@ -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
Expand Down
26 changes: 13 additions & 13 deletions src/daemon/Display.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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))
Expand Down
14 changes: 9 additions & 5 deletions src/daemon/Display.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{};
Expand Down Expand Up @@ -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:
Expand Down
13 changes: 11 additions & 2 deletions src/daemon/SeatManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Loading
Loading