Skip to content

fix(session): store logind session id as string to avoid collision #1465

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

fix(session): store logind session id as string to avoid collision #1465

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
17 changes: 9 additions & 8 deletions src/greeter/greeterproxy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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();
Expand All @@ -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;
Comment thread
wineee marked this conversation as resolved.
});
} else {
QMetaObject::invokeMethod(this, [this] {
Expand Down Expand Up @@ -530,7 +531,7 @@ void GreeterProxy::readyRead()
#endif
case DaemonMessages::UserActivateMessage: {
QString user;
int sessionId;
QString sessionId;
Comment thread
zccrs marked this conversation as resolved.
input >> user >> sessionId;

// NOTE: maybe DDM will active dde user.
Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/greeter/usermodel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/seat/helper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<WXdgDecorationManager>();
connect(m_xdgDecorationManager,
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/seat/helper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
34 changes: 23 additions & 11 deletions src/session/session.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
}
}

int Session::id() const
const QString &Session::id() const
Comment thread
wineee marked this conversation as resolved.
{
return m_id;
}
Expand Down Expand Up @@ -224,13 +224,15 @@
*
* @param newEnabled New enabled state
*/
void SessionManager::setActiveSocketEnabled(bool newEnabled)

Check warning on line 227 in src/session/session.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

The function 'setActiveSocketEnabled' is never used.
{
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!";
}
}

/**
Expand Down Expand Up @@ -264,7 +266,7 @@
* @param username Username to ensure session for
* @returns Session for the given username, or nullptr on failure
*/
std::shared_ptr<Session> SessionManager::ensureSession(int id, QString username)
std::shared_ptr<Session> SessionManager::ensureSession(const QString &id, QString username)
{
// Helper lambda to create WSocket and WXWayland
auto createWSocket = [this]() {
Expand Down Expand Up @@ -419,10 +421,10 @@
* @param id Session ID to find session for
* @returns Session for the given id, or nullptr if not found
*/
std::shared_ptr<Session> SessionManager::sessionForId(int id) const
std::shared_ptr<Session> SessionManager::sessionForId(const QString &id) const

Check warning on line 424 in src/session/session.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

The function 'sessionForId' is never used.
{
for (const auto &session : std::as_const(m_sessions)) {
if (session && session->m_id == id)

Check warning on line 427 in src/session/session.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

Consider using std::find_if algorithm instead of a raw loop.
return session;
}
return nullptr;
Expand Down Expand Up @@ -488,9 +490,17 @@
return nullptr;
}

bool SessionManager::isDDEUserClient(WClient *client)

Check warning on line 493 in src/session/session.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

The function 'isDDEUserClient' is never used.
{
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()
Expand Down Expand Up @@ -526,13 +536,13 @@
*
* @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)

Check warning on line 539 in src/session/session.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

The function 'updateActiveUserSession' is never used.
{
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();
Expand All @@ -551,8 +561,10 @@
// 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);
}

Expand Down
15 changes: 9 additions & 6 deletions src/session/session.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Session : public QObject {
public:
~Session();

int id() const;
const QString &id() const;
Comment thread
wineee marked this conversation as resolved.
uid_t uid() const;
const QString &username() const;
WSocket *socket() const;
Expand All @@ -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;
Expand Down Expand Up @@ -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> session);
std::shared_ptr<Session> sessionForId(int id) const;
std::shared_ptr<Session> sessionForId(const QString &id) const;
std::shared_ptr<Session> sessionForUid(uid_t uid) const;
std::shared_ptr<Session> sessionForUser(const QString &username) const;
std::shared_ptr<Session> sessionForXWayland(WXWayland *xwayland) const;
Expand All @@ -92,7 +95,7 @@ class SessionManager : public QObject {
void xwaylandAuthChanged();

private:
std::shared_ptr<Session> ensureSession(int id, QString username);
std::shared_ptr<Session> ensureSession(const QString &id, QString username);

std::weak_ptr<Session> m_activeSession;
QList<std::shared_ptr<Session>> m_sessions;
Expand Down
Loading