[FFe] gamemode

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.

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?

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.

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

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

@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.

@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?

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

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> ]

> 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

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)

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.

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

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.

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.

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