[FFe] gamemode

Bug #1853830 reported by Martin Wimpress  on 2019-11-25
22
This bug affects 2 people
Affects Status Importance Assigned to Milestone
gamemode (Ubuntu)
Undecided
Sebastien Bacher
ubuntu-mate-meta (Ubuntu)
Low
Martin Wimpress 

Bug Description

[Availability]

Builds on amd64 only in Ubuntu and syncs from Debian. GameMode was designed primarily as a solution to the Intel and AMD CPU powersave or ondemand governors, and intended for gaming; hence why it is amd64 only.

[Rationale]

We would like to enable GameMode as it allows games to request a set of optimisations be temporarily applied to the host OS and improve gaming performance.

The Ubuntu desktop team intend to seed gamemode.

[Security]

No CVE/known security issue. GameMode includes an user daemon that modifies the CPU governors and GPU clock state through pkexec-ed helpers.

[Quality assurance]

- The desktop-packages team is subscribed to the package
- 2 bugs in Debian BTS which are being investigated. None in LP.
- The package is actively maintained in Debian and in sync in Ubuntu.
- No test suite, but difficult to integrate with autopkgtest due to hardware requirements.

[Dependencies]

All the Depends: are currently in main. There are no Recommends:

- init-system-helpers (>= 1.52)
- libc6 (>= 2.27)
- libsystemd0 (>= 221)
- libdbus-1-3 (>= 1.9.14)

[Standards compliance]

The package is using current (dh12) standards, the standards-version is 4.4.0, the package is in sync from Debian.

[Maintenance]

Upstream is active and the desktop team is going to look after the package in Ubuntu.

description: updated
description: updated
description: updated
Didier Roche (didrocks) on 2020-01-14
Changed in gamemode (Ubuntu):
assignee: nobody → Didier Roche (didrocks)
Didier Roche (didrocks) wrote :

Good packaging practice: despite the missing trailing comma, dh has fail-missing, la files are not shipped, upstream is called with --as-needed. Hardening flags are enabled.

1/ One lintian warning that I’m fine to ignore (but will be better to fix it in lintian or have an override): W: gamemode source: incorrect-packaging-filename debian/TODO.debian -> debian/TODO

inih has been devendorized. OK

2/ I know that the soname is still at 0. However, I would like to have symbols files present with makeshlibs called -c4 for both libs please. I find it concerning now that release 1.X is reached, we still have an unstable ABI and it should be tracked. I saw there is an explicit lintian override for skipping them. Is there any reasons which is still valid and we don’t want to track the API?

3/ There are now 2 (very recent, I admit :p) bugs in launchpad. I think bug #1862455 should be investigated while the package should be updated to 1.5 instead of a git snapshot (bug #1863570).

4/ The daemon is executed as root from a user process which talks to the daemon via dbus and authenticate via polkit. Note that the dbus activated service or the user activated service have different options:
- The -d option is fine as the dbus activated is called with a daemonizing options but not the systemd one (prefered way of working if activated by systemd)
- However, I don’t understand why the systemd service is called with -l (log to syslog), which is out default for systemd service (the journal forwards to syslog, even for user services). The day we want to remove syslog completely, it will prevent having to patch it.

5/ I am not completely sure why the 2 -dev are separated. However, as the only libgamemodeauto-dev ships some headers, are you sure that we can build against libgamemodeauto0 without having those headers installed? If not, libgamemodeauto-dev should dep on libgamemode-dev.

6/ I think libmode* should recommends, provides some alternative or at least suggests the package having the daemon.

When those are fixed (in particular the update to 1.5), I would like to request a security review as this daemon is root executed by an user command.

Didier Roche (didrocks) on 2020-02-19
Changed in gamemode (Ubuntu):
status: New → Incomplete
Sebastien Bacher (seb128) wrote :

Hey Didier, thanks for the review!

> 1/ One lintian warning that I’m fine to ignore (but will be better to fix it in lintian or have an override):
> W: gamemode source: incorrect-packaging-filename debian/TODO.debian -> debian/TODO

Proposed fix to Debian in https://salsa.debian.org/games-team/gamemode/merge_requests/7

> 2/ I know that the soname is still at 0. However, I would like to have symbols files present with makeshlibs called -c4 for both libs please.
> I find it concerning now that release 1.X is reached, we still have an unstable ABI and it should be tracked.
> I saw there is an explicit lintian override for skipping them. Is there any reasons which is still valid and we don’t want to track the API?

I opened a bug on the BTS to start the discussion about that
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=952425

> 3/ There are now 2 (very recent, I admit :p) bugs in launchpad.

The first one was about 19.10 and fixed in focal (the packaging was buggy), the 1.5 update was done in Debian over the w.e and synced to focal now.

> - However, I don’t understand why the systemd service is called with -l (log to syslog), which is out default for systemd service
> (the journal forwards to syslog, even for user services). The day we want to remove syslog completely, it will prevent having to patch it.

Reported with a mp to frop the -l upstream on https://github.com/FeralInteractive/gamemode/pull/198

> 5/ I am not completely sure why the 2 -dev are separated. However, as the only libgamemodeauto-dev ships some headers,
> are you sure that we can build against libgamemodeauto0 without having those headers installed?
> If not, libgamemodeauto-dev should dep on libgamemode-dev.
...
> 6/ I think libmode* should recommends, provides some alternative or at least suggests the package having the daemon.

I will have a look to those/talk to the Debian

> When those are fixed (in particular the update to 1.5), I would like to request a security review as this daemon is root executed by an user command.

We got the 1.5 update done, the bug closed and the ball rolling on the other issues, I guess that's good enough to bounce to security while the remaining packaging questions are discussed?

Changed in gamemode (Ubuntu):
status: Incomplete → New
Didier Roche (didrocks) wrote :

Thanks for starting the discussions with debian!

> We got the 1.5 update done, the bug closed and the ball rolling on the
other issues, I guess that's good enough to bounce to security while the
remaining packaging questions are discussed?

Agreed. It’s up for security review code-wise and we can follow up on the rest meanwhile.

Changed in gamemode (Ubuntu):
assignee: Didier Roche (didrocks) → Canonical Security Team (canonical-security)
Rick (rickandtired) wrote :

A 32bit build would be greatly useful for use with Lutris.
Lutris has a gamemode toggle in their program, but it requires both 32bit and 64bit gamemode

Steve Beattie (sbeattie) on 2020-02-26
Changed in gamemode (Ubuntu):
assignee: Canonical Security Team (canonical-security) → Ubuntu Security Team (ubuntu-security)
Seth Arnold (seth-arnold) wrote :

I reviewed gamemode 1.5-1 as checked into focal. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

gamemode tries to improve the Linux gaming experience by switching to more
reliable CPU governors, rescheduling processes, changing io priorities,
inhibiting screensaver, etc.

- CVE History:
  - No CVEs; upstream responded to an issue I filed very quickly
- Build-Depends: debhelper-compat, git, libdbus-1-dev, libinih-dev,
  libinih1, libsystemd-dev, meson, ninja-build, pkg-config, systemd
- pre/post inst/rm scripts automatically generated
- no init scripts
- systemd unit starts gamemode daemon when the dbus binding is needed
- dbus unit starts gamemode daemon when the dbus binding is needed
- no setuid binaries
- binaries gamemoded and gamemoderun
- no sudo fragments
- polkit file: allows active users to run cpugovctl and gpuclockctl
- no udev rules
- tests are not run during build, probably they do not belong on the
  build; unknown if they work well enough for autopkgtest, but they look
  promising.
- no cron jobs
- Build logs:
 W: gamemode source: debhelper-compat-file-is-missing
 W: gamemode source: package-uses-deprecated-debhelper-compat-version 1
 E: gamemode source: package-uses-debhelper-but-lacks-build-depends
 E: gamemode source: missing-build-dependency debhelper
 W: gamemode source: newer-standards-version 4.5.0 (current is 4.1.4)

Probably the last warning can be ignored.

- Processes spawned safely
- Memory management looks simple, sane
- File IO paths and contents looked safe enough; some assumptions were
  made about how much data the kernel ABI files will return but these are
  probably safe assumptions to make.
- Logging looked safe
- Environment variables looked safe
- No privileged functions, but some privileged kernel operations
- No cryptography
- No temp files
- Networking only via dbus
- No use of webkit
- Provides a polkit backend

- cppcheck only one false positive
  - SEE cppcheck.txt
- many coverity false positives, a few legit findings of small value
- no shellcheck results in shipped code

The issue I filed was responded to very quickly:

https://github.com/FeralInteractive/gamemode/issues/203

And the handful of issues that looked real from Coverity:

game_mode_resolve_wine_preloader() proc_fd = INVALID_PROCFD causes a
game_mode_close_proc(-1) call.

get_gov_state() the ftell(3) call could return -1, which would give a bad
contents VLA and bad input to fread(3).

daemonize() if the open("/dev/null") calls fail, dup2(2) and close(2) are
given bad inputs

Honestly these are all pretty low impact.

I filed https://github.com/FeralInteractive/gamemode/issues/206 for these
issues.

Security team ACK for promoting gamemode to main.

Thanks

Changed in gamemode (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody

@seb128/didrocks - will you two work together to bring the requested changes on the MIR review to an acceptable state - until didrocks does accept them? I'll assign to seb to make this clear.
Once that is done as usual set it to "in Progress" and seed/depend on it and ping an AA.

Changed in gamemode (Ubuntu):
assignee: nobody → Sebastien Bacher (seb128)
Sebastien Bacher (seb128) wrote :

@Didier,

This new upload https://launchpad.net/ubuntu/+source/gamemode/1.5.1-0ubuntu1 includes
- the fix for the logging option default
- .symbols and corresponding debian/rules check to stop build on api changes
- the auto-dev depends on the library, I will still follow up in Debian to get the libs merged

I think with that all the MIR reviews point at approved, could you check/confirm it's true for you and give the MIR team ack?

Didier Roche (didrocks) wrote :

Everything looks good with this upload (well, very very small nitpick: no trailing comma on recommend, but I will take a breath and survive :)).
Ack from the MIR perspective

Changed in gamemode (Ubuntu):
status: New → Fix Committed
summary: - [MIR] gamemode
+ [FFe] gamemode

On Tue, Mar 24, 2020 at 09:26:30AM -0000, Launchpad Bug Tracker wrote:
> You have been subscribed to a public bug by Martin Wimpress (flexiondotorg):

I've not been following this particularly closely, but this bug looks
like an ex-MIR which needs some more information for feature freeze
consideration. What is the feature freeze exception for? Seeding onto
the ISO presumably?

  - What testing has been done? Given that it's being added late,
    there's less time for more organic testing to happen.
  - What's the risk to users: where might problems appear for them so
    what should we be keeping half an eye on?

I noticed that this adds a new daemon to be run by each user all of the
time (it is activated from systemd --user's default.target), whether
they have a graphical session or not or want to ever make use of this
functionality. Is this justified? It looks like the program actually
exposes a D-Bus interface and is activatable - would it be possible
please to investigate if the systemd unit can be not enabled (but still
installed) so that people only get this running once something on the
system asks for it?

Cheers,

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

Sebastien Bacher (seb128) wrote :

> exposes a D-Bus interface and is activatable - would it be possible
> please to investigate if the systemd unit can be not enabled (but still
> installed) so that people only get this running once something on the

Thanks for pointing that out, we got bitten again by dh_installsystemduser auto-enabling units. I've fixed it now in that upload by using --no-enable (and migrating upgraders)

https://launchpad.net/ubuntu/+source/gamemode/1.5.1-0ubuntu2

I've confirmed that gamemode isn't active on login after the change but gamemode-simulate-game does dbus activate and the schedule mode is correctly changed

In summary that point should be addressed now!

@Martin, can you handle the FFe side of the issue?

Here is the test case I've used; I'm using glmark2 as "the game".

    sudo apt install gamemode glmark2

Reboot.

Log in and check that gamemode is not running by default:

    ps -efH | grep gamemode | grep -v grep

Check what CPU `scaling_governor` is enabled by default, it should report `powersave`

    cat /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
    powersave
    powersave
    powersave
    powersave

Invoke gamemode.

    gamemoderun glmark2

This should have activated `gamemoded`, check the process has been started:

    ps -efH | grep gamemode | grep -v grep

While the game is running check what CPU `scaling_governor` is active, it should report `performance`.

    cat /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
    performance
    performance
    performance
    performance

Quit the game and check the CPU `scaling_governor` again, it should now report `powersave`.

    cat /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor
    powersave
    powersave
    powersave
    powersave

I have also used the GameMode extension for GNOME and it correctly reports when gamemode has been activated.

  * https://github.com/gicmo/gamemode-extension

Matthew Keeler (mttke) wrote :

Tested twice, both times it only showed Powersave while the game was running.
This is with Intel internal graphics (CPU: i5-4690k)
(And for clarification: after invoking gamemode, I had to open a second terminal to check the processes)

Iain Lane (laney) wrote :

OK, please go ahead.

I would appreciate it if a group of people were to test this feature out and make sure that it doesn't have any unintentional side-effects and that it works (c.f. comment #12). (I think Martin is organising this already.) But please address those further issues in separate bug reports.

Martin, I would appreciate it if you would personally subscribe to gamemode bugs at least for a little while, and make sure that any regressions in the wider desktop are attended to.

Darin Miller (darinmiller) wrote :

Gamemode worked perfectly following Wimpy's guidelines in comment #11 using Kubuntu 20.04.

Bill (franksmcb) (franksmcb) wrote :

Tested with T430 using nVidia running Ubuntu MATE

Ran testcase provided in #11.

Before running: powersave
When running gamemode I am seeing: performance
After stopping: powersave

I have personally subscribed to all gamemode bugs.

Changed in ubuntu-mate-meta (Ubuntu):
status: New → Triaged
assignee: nobody → Martin Wimpress (flexiondotorg)
importance: Undecided → Low
Iain Lane (laney) wrote :

Please could everybody keep testing results out of this bug, which is just for getting gamemode in the default install?

If you have a *bug*, feel free to open a new report, of course.

Changed in ubuntu-mate-meta (Ubuntu):
status: Triaged → In Progress
Changed in ubuntu-mate-meta (Ubuntu):
status: In Progress → Fix Committed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-mate-meta - 1.263

---------------
ubuntu-mate-meta (1.263) focal; urgency=medium

  * Refreshed dependencies
  * Added gamemode to core-recommends [amd64], desktop-recommends
    [amd64] (LP: #1853830)

 -- Martin Wimpress <email address hidden> Sun, 12 Apr 2020 11:35:14 +0100

Changed in ubuntu-mate-meta (Ubuntu):
status: Fix Committed → Fix Released
Changed in gamemode (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.