Avoid undefined behavior due to dangling file descriptor
ClosedPublic

Authored by tobiasdeiminger on Sep 17 2018, 12:32 PM.

Details

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

Diff Detail

Repository
R223 Okular
Branch
fix_dbus_fd
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2955
Build 2973: arc lint + arc unit
Restricted Application added a project: Okular. · View Herald TranscriptSep 17 2018, 12:32 PM
Restricted Application added a subscriber: okular-devel. · View Herald Transcript
tobiasdeiminger requested review of this revision.Sep 17 2018, 12:32 PM
anthonyfieroni added inline comments.
ui/presentationwidget.cpp
1763

It should be != -1

Fix check for m_sleepInhibitFd

tobiasdeiminger marked an inline comment as done.Sep 17 2018, 1:02 PM
tobiasdeiminger added inline comments.
ui/presentationwidget.cpp
1763

You're right, thanks

aacid accepted this revision.Sep 18 2018, 6:00 PM

Awesome find!

I guess i should close the pulse/gstreamer bugs :D

Please commit to stable branch and merge to master

This revision is now accepted and ready to land.Sep 18 2018, 6:00 PM
This revision was automatically updated to reflect the committed changes.
tobiasdeiminger marked an inline comment as done.