[plasma/plasma-login-manager] src: Hold VT references properly
Oliver Beard <[email protected]>
| Newsgroups | gmane.comp.kde.cvs |
|---|---|
| Message-ID | <[email protected]> |
Git commit 107caaa87cb1da55d86cf77376c3c8ce6c0374f1 by Oliver Beard, on behalf of David Edmundson.
Committed on 23/07/2026 at 14:34.
Pushed by davidedmundson into branch 'master'.
Hold VT references properly
The VT_OPENQRY call returns a VT that is not in use by querying if that
TTY is open.
The way this should work is we query for an unused VT, then hold a
reference to it all the way until the session is opened.
In part of the wayland/rootlesss X refactoring the active VT
then not actually open it until the user session finally spawns.
We don't pass the FD to the UserSession, it's safe for us to just open
the VT again. The important part is that someone having a reference
limits it being available in future VT_OPENQRY calls.
The FileDescriptor class is copied verbatim from kwin.
M +1 -0 src/common/CMakeLists.txt
M +41 -11 src/common/VirtualTerminal.cpp
M +19 -1 src/common/VirtualTerminal.h
A +94 -0 src/common/filedescriptor.cpp [License: GPL(v2.0+)]
A +43 -0 src/common/filedescriptor.h [License: GPL(v2.0+)]
M +6 -6 src/daemon/Display.cpp
M +3 -2 src/daemon/Display.h
M +3 -3 src/daemon/Seat.cpp
M +1 -1 src/daemon/Seat.h
https://invent.kde.org/plasma/plasma-login-manager/-/commit/107caaa87cb1da55d86cf77376c3c8ce6c0374f1
diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt
index 581ab18c..f3d268b3 100644
--- a/src/common/CMakeLists.txt
+++ b/src/common/CMakeLists.txt
@@ -1,6 +1,7 @@
configure_file(${CMAKE_CURRENT_SOURCE_DIR}/Constants.h.in ${CMAKE_CURRENT_BINARY_DIR}/Constants.h IMMEDIATE @ONLY)
add_library(plasmalogin-common OBJECT
+ filedescriptor.cpp
SafeDataStream.cpp
Session.cpp
SocketWriter.cpp
diff --git a/src/common/VirtualTerminal.cpp b/src/common/VirtualTerminal.cpp
index 8522128c..6c1e7737 100644
--- a/src/common/VirtualTerminal.cpp
+++ b/src/common/VirtualTerminal.cpp
@@ -28,6 +28,7 @@
#include <string.h>
#include <sys/ioctl.h>
#include <unistd.h>
+#include <utility>
#define RELEASE_DISPLAY_SIGNAL (SIGRTMAX)
#define ACQUIRE_DISPLAY_SIGNAL (SIGRTMAX - 1)
@@ -38,6 +39,27 @@ namespace VirtualTerminal
{
const char *defaultVtPath = "/dev/tty0";
+Terminal::Terminal(int tty, FileDescriptor ttyFd)
+ : m_tty(tty)
+ , m_ttyFd(std::move(ttyFd))
+{
+}
+
+bool Terminal::isValid() const
+{
+ return m_tty > 0;
+}
+
+int Terminal::tty() const
+{
+ return m_tty;
+}
+
+const FileDescriptor &Terminal::ttyFd() const
+{
+ return m_ttyFd;
+}
+
QString path(int vt)
{
return QStringLiteral("/dev/tty%1").arg(vt);
@@ -171,13 +193,24 @@ int currentVt()
return getVtActive(fd);
}
-int setUpNewVt()
+Terminal openVt(int vt)
+{
+ const QString ttyPath = path(vt);
+ const int fd = open(qPrintable(ttyPath), O_RDWR | O_NOCTTY | O_CLOEXEC);
+ if (fd < 0) {
+ qWarning() << "Failed to open" << ttyPath << ':' << strerror(errno);
+ return {};
+ }
+ return {vt, FileDescriptor(fd)};
+}
+
+Terminal setUpNewVt()
{
// open VT master
int fd = open(defaultVtPath, O_RDWR | O_NOCTTY);
if (fd < 0) {
qCritical() << "Failed to open VT master:" << strerror(errno);
- return -1;
+ return {};
}
auto closeFd = qScopeGuard([fd] {
close(fd);
@@ -185,18 +218,15 @@ int setUpNewVt()
int vt = 0;
if (ioctl(fd, VT_OPENQRY, &vt) < 0) {
- qCritical() << "Failed to open new VT:" << strerror(errno);
- return -1;
+ qCritical() << "Failed to find a new VT:" << strerror(errno);
+ return {};
}
- // fallback to active VT
- if (vt <= 0) {
- int vtActive = getVtActive(fd);
- qWarning() << "New VT" << vt << "is not valid, fall back to" << vtActive;
- return vtActive;
+ Terminal terminal = openVt(vt);
+ if (!terminal.isValid()) {
+ qCritical() << "Failed to open a new VT";
}
-
- return vt;
+ return terminal;
}
void jumpToVt(int vt, bool vt_auto)
diff --git a/src/common/VirtualTerminal.h b/src/common/VirtualTerminal.h
index 874af5d8..09eb11b1 100644
--- a/src/common/VirtualTerminal.h
+++ b/src/common/VirtualTerminal.h
@@ -19,15 +19,33 @@
#include <QString>
+#include "filedescriptor.h"
+
namespace PLASMALOGIN
{
namespace VirtualTerminal
{
extern const char *defaultVtPath;
+class Terminal
+{
+public:
+ Terminal() = default;
+ Terminal(int tty, FileDescriptor ttyFd);
+
+ bool isValid() const;
+ int tty() const;
+ const FileDescriptor &ttyFd() const;
+
+private:
+ int m_tty = -1;
+ FileDescriptor m_ttyFd;
+};
+
QString path(int vt);
int currentVt();
-int setUpNewVt();
+Terminal openVt(int vt);
+Terminal setUpNewVt();
void jumpToVt(int vt, bool vt_auto);
void ignoreVtSwitches();
}
diff --git a/src/common/filedescriptor.cpp b/src/common/filedescriptor.cpp
new file mode 100644
index 00000000..ee18bf6f
--- /dev/null
+++ b/src/common/filedescriptor.cpp
@@ -0,0 +1,94 @@
+/*
+ SPDX-FileCopyrightText: 2022 Xaver Hugl <[email protected]>
+
+ SPDX-License-Identifier: GPL-2.0-or-later
+*/
+#include "filedescriptor.h"
+
+#include <QDebug>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <string.h>
+#include <sys/poll.h>
+#include <unistd.h>
+#include <utility>
+
+namespace PLASMALOGIN
+{
+
+FileDescriptor::FileDescriptor(int fd)
+ : m_fd(fd)
+{
+}
+
+FileDescriptor::FileDescriptor(FileDescriptor &&other)
+ : m_fd(std::exchange(other.m_fd, -1))
+{
+}
+
+FileDescriptor &FileDescriptor::operator=(FileDescriptor &&other)
+{
+ reset();
+ m_fd = std::exchange(other.m_fd, -1);
+ return *this;
+}
+
+FileDescriptor::~FileDescriptor()
+{
+ reset();
+}
+
+bool FileDescriptor::isValid() const
+{
+ return m_fd != -1;
+}
+
+int FileDescriptor::get() const
+{
+ return m_fd;
+}
+
+int FileDescriptor::take()
+{
+ return std::exchange(m_fd, -1);
+}
+
+void FileDescriptor::reset()
+{
+ if (m_fd != -1) {
+ if (::close(m_fd) != 0) {
+ qWarning() << "Failed to close file descriptor:" << strerror(errno);
+ }
+ m_fd = -1;
+ }
+}
+
+FileDescriptor FileDescriptor::duplicate() const
+{
+ return m_fd != -1 ? FileDescriptor{fcntl(m_fd, F_DUPFD_CLOEXEC, 0)} : FileDescriptor{};
+}
+
+bool FileDescriptor::isClosed() const
+{
+ return isClosed(m_fd);
+}
+
+bool FileDescriptor::isReadable() const
+{
+ return isReadable(m_fd);
+}
+
+bool FileDescriptor::isClosed(int fd)
+{
+ pollfd pfd = {.fd = fd, .events = POLLIN, .revents = 0};
+ return poll(&pfd, 1, 0) < 0 || pfd.revents & (POLLHUP | POLLERR);
+}
+
+bool FileDescriptor::isReadable(int fd)
+{
+ pollfd pfd = {.fd = fd, .events = POLLIN, .revents = 0};
+ return poll(&pfd, 1, 0) && (pfd.revents & (POLLIN | POLLNVAL)) != 0;
+}
+
+}
diff --git a/src/common/filedescriptor.h b/src/common/filedescriptor.h
new file mode 100644
index 00000000..c94624c8
--- /dev/null
+++ b/src/common/filedescriptor.h
@@ -0,0 +1,43 @@
+/*
+ SPDX-FileCopyrightText: 2022 Xaver Hugl <[email protected]>
+
+ SPDX-License-Identifier: GPL-2.0-or-later
+*/
+#pragma once
+
+namespace PLASMALOGIN
+{
+
+class FileDescriptor
+{
+public:
+ FileDescriptor() = default;
+ explicit FileDescriptor(int fd);
+ FileDescriptor(FileDescriptor &&);
+ FileDescriptor &operator=(FileDescriptor &&);
+ ~FileDescriptor();
+
+ explicit operator bool() const;
+
+ bool isValid() const;
+ int get() const;
+ int take();
+ void reset();
+ FileDescriptor duplicate() const;
+
+ bool isReadable() const;
+ bool isClosed() const;
+
+ static bool isReadable(int fd);
+ static bool isClosed(int fd);
+
+private:
+ int m_fd = -1;
+};
+
+inline FileDescriptor::operator bool() const
+{
+ return isValid();
+}
+
+}
diff --git a/src/daemon/Display.cpp b/src/daemon/Display.cpp
index 875ceaf1..c8d53fd6 100644
--- a/src/daemon/Display.cpp
+++ b/src/daemon/Display.cpp
@@ -58,7 +58,7 @@ Display::Display(Seat *parent)
if (seat()->canTTY()) {
m_terminalId = seat()->availableVt();
}
- qDebug("Using VT %d", m_terminalId);
+ qDebug("Using VT %d", m_terminalId.tty());
// respond to authentication requests
m_auth->setVerbose(true);
@@ -138,7 +138,7 @@ Display::~Display()
int Display::terminalId() const
{
- return m_auth->isActive() ? m_sessionTerminalId : m_terminalId;
+ return (m_auth->isActive() ? m_sessionTerminalId : m_terminalId).tty();
}
QString Display::sessionType() const
@@ -283,7 +283,7 @@ bool Display::startAuth(const QString &user, const QString &password, const Sess
// last session later, in slotAuthenticationFinished()
m_sessionName = session.fileName();
- m_sessionTerminalId = m_terminalId;
+ m_sessionTerminalId = {m_terminalId.tty(), m_terminalId.ttyFd().duplicate()};
if (m_greeter->isRunning()) {
// Create a new VT when we need to have another compositor running
@@ -293,7 +293,7 @@ bool Display::startAuth(const QString &user, const QString &password, const Sess
}
// some information
- qDebug() << "Session" << m_sessionName << "selected, command:" << session.exec() << "for VT" << m_sessionTerminalId << session.xdgSessionType();
+ qDebug() << "Session" << m_sessionName << "selected, command:" << session.exec() << "for VT" << m_sessionTerminalId.tty() << session.xdgSessionType();
QProcessEnvironment env;
env.insert(QStringLiteral("PATH"), PlasmaLogin::config()->defaultPath());
@@ -306,8 +306,8 @@ bool Display::startAuth(const QString &user, const QString &password, const Sess
env.insert(QStringLiteral("XDG_SESSION_CLASS"), QStringLiteral("user"));
env.insert(QStringLiteral("XDG_SESSION_TYPE"), session.xdgSessionType());
env.insert(QStringLiteral("XDG_SEAT"), seat()->name());
- if (m_sessionTerminalId > 0) {
- env.insert(QStringLiteral("XDG_VTNR"), QString::number(m_sessionTerminalId));
+ if (m_sessionTerminalId.isValid()) {
+ env.insert(QStringLiteral("XDG_VTNR"), QString::number(m_sessionTerminalId.tty()));
}
env.insert(QStringLiteral("XDG_SESSION_DESKTOP"), session.desktopNames());
diff --git a/src/daemon/Display.h b/src/daemon/Display.h
index c267e3db..a04b6b81 100644
--- a/src/daemon/Display.h
+++ b/src/daemon/Display.h
@@ -25,6 +25,7 @@
#include "Auth.h"
#include "Session.h"
+#include "VirtualTerminal.h"
class QLocalSocket;
@@ -75,8 +76,8 @@ private:
bool m_started{false};
- int m_terminalId = -1;
- int m_sessionTerminalId = 0;
+ VirtualTerminal::Terminal m_terminalId;
+ VirtualTerminal::Terminal m_sessionTerminalId;
QString m_passPhrase;
QString m_sessionName;
diff --git a/src/daemon/Seat.cpp b/src/daemon/Seat.cpp
index 515a079f..c365afad 100644
--- a/src/daemon/Seat.cpp
+++ b/src/daemon/Seat.cpp
@@ -76,15 +76,15 @@ bool Seat::isTtyInUse(const QString &tty) const
return false;
}
-int Seat::availableVt() const
+VirtualTerminal::Terminal Seat::availableVt() const
{
if (!isTtyInUse(QStringLiteral("tty%1").arg(PLASMALOGIN_INITIAL_VT))) {
- return PLASMALOGIN_INITIAL_VT;
+ return VirtualTerminal::openVt(PLASMALOGIN_INITIAL_VT);
}
const auto vt = VirtualTerminal::currentVt();
if (vt > 0 && !isTtyInUse(QStringLiteral("tty%1").arg(vt))) {
- return vt;
+ return VirtualTerminal::openVt(vt);
}
return VirtualTerminal::setUpNewVt();
diff --git a/src/daemon/Seat.h b/src/daemon/Seat.h
index 1f562cdd..9b890d5d 100644
--- a/src/daemon/Seat.h
+++ b/src/daemon/Seat.h
@@ -37,7 +37,7 @@ public:
void createDisplay();
bool canTTY();
bool tryLockFirstLogin();
- int availableVt() const;
+ VirtualTerminal::Terminal availableVt() const;
QString reusableSessionId(const QString &user) const;
void activateSession(const QString &sessionId) const;
std::optional<int> vtForSession(const QString &sessionId) const;