Avoid undefined behavior due to dangling file descriptor

Summary:
We request a inhibit lock of DBus type UNIX_FD from systemd logind. It's wrapped into (and owned by) a QDBusUnixFileDescriptor object of automatic storage. The file descriptor will be closed in QDBusUnixFileDescriptor Dtor, and may be reused by some other facility (e.g. pulseaudio).

If we want to store the lock longer than QDBusUnixFileDescriptor lifetime, we have to dup the file descriptor. If we don't dup, and close the original fd later in PresentationWidget::allowPowerManagement, bad things may happen.

Besides that, what we get from systemd is really a file descriptor, not a "cookie". So I renamed the m_sleepInhibitCookie to m_sleepInhibitFd and changed initial state to -1 accordingly.

BUG 393478
BUG 398720

Test Plan:
- bugs don't occur any longer
- inhibiting sleep during presentation mode still works

Reviewers: aacid

Reviewed By: aacid

Subscribers: ngraham, anthonyfieroni, okular-devel

Tags: #okular

Differential Revision: https://phabricator.kde.org/D15574
This commit is contained in:
Tobias Deiminger 2018-09-18 21:30:25 +02:00
parent 8ff7abc14d
commit fa7a1b3d06
2 changed files with 7 additions and 7 deletions

View file

@ -142,7 +142,7 @@ class PresentationToolBar : public QToolBar
PresentationWidget::PresentationWidget( QWidget * parent, Okular::Document * doc, DrawingToolActions * drawingToolActions, KActionCollection * collection )
: QWidget( nullptr /* must be null, to have an independent widget */, Qt::FramelessWindowHint ),
m_pressedLink( nullptr ), m_handCursor( false ), m_drawingEngine( nullptr ),
m_screenInhibitCookie(0), m_sleepInhibitCookie(0),
m_screenInhibitCookie(0), m_sleepInhibitFd(-1),
m_parentWidget( parent ),
m_document( doc ), m_frameIndex( -1 ), m_topBar( nullptr ), m_pagesEdit( nullptr ), m_searchBar( nullptr ),
m_ac( collection ), m_screenSelect( nullptr ), m_isSetup( false ), m_blockNotifications( false ), m_inBlackScreenMode( false ),
@ -1735,7 +1735,7 @@ void PresentationWidget::inhibitPowerManagement()
}
}
if (!m_sleepInhibitCookie) {
if (m_sleepInhibitFd != -1) {
QDBusMessage message = QDBusMessage::createMethodCall(QStringLiteral("org.freedesktop.login1"),
QStringLiteral("/org/freedesktop/login1"),
QStringLiteral("org.freedesktop.login1.Manager"),
@ -1749,7 +1749,7 @@ void PresentationWidget::inhibitPowerManagement()
QDBusPendingReply<QDBusUnixFileDescriptor> reply = QDBusConnection::systemBus().asyncCall(message);
reply.waitForFinished();
if (reply.isValid()) {
m_sleepInhibitCookie = reply.value().fileDescriptor();
m_sleepInhibitFd = dup(reply.value().fileDescriptor());
} else {
qCWarning(OkularUiDebug) << "Unable to inhibit sleep" << reply.error();
}
@ -1760,9 +1760,9 @@ void PresentationWidget::inhibitPowerManagement()
void PresentationWidget::allowPowerManagement()
{
#ifdef Q_OS_LINUX
if (m_sleepInhibitCookie) {
::close(m_sleepInhibitCookie);
m_sleepInhibitCookie = 0;
if (m_sleepInhibitFd != -1) {
::close(m_sleepInhibitFd);
m_sleepInhibitFd = -1;
}
if (m_screenInhibitCookie) {

View file

@ -114,7 +114,7 @@ class PresentationWidget : public QWidget, public Okular::DocumentObserver
QRect m_drawingRect;
int m_screen;
uint m_screenInhibitCookie;
int m_sleepInhibitCookie;
int m_sleepInhibitFd;
// transition related
QTimer * m_transitionTimer;