MIR: vulkan-loader

[Teg]

[Availability]
Has been in universe since March '16

[Rationale]
mesa-vulkan-drivers depends on libvulkan1, and once m-v-d is added to xserver-xorg Recommends libvulkan1 should want to migrate (but apparently didn't).

Also gtk4 and qt5 use vulkan now. It is intended to demote qt to universe for 19.04. gtk4 is not expected to enter Ubuntu main until 19.10 (or maybe later).

[Security]
No known security issues in the past

[Quality assurance]
- doesn't need any configuration
- doesn't have any debconf questions
- all existing bugs are or should be fixed by new packaging in NEW
- maintained by xorg-team on Debian, synced to Ubuntu
- has build-time tests
- debian/watch is used
- uses python3

[Dependencies]
all in main

[Standards compliance]
is compliant

[Maintenance]
maintained by xorg-team on salsa.debian.org, synced to Ubuntu

[Background information]
upstream split the source in four, old src:vulkan is going away and src:vulkan-loader will be the one to be promoted once it's accepted from NEW.

[original description]

As of January 2018, the Vulkan packages (libvulkan1, libvulkan-dev, vulkan-utils) are currently in the Universe repository instead of Main for Ubuntu 18.04. Since the Vulkan packages are in a mature state and with future GTK and Qt development looking to adapt Vulkan, it would be important to have the Vulkan packages available.

This change will affect game developers the most as they will know that Vulkan will be available to them for future development. There has been a lot of movement in Mesa 13.3 and above with the RADV driver where it has matched and exceeded the non-free alternatives. It is essential to get improved Vulkan support for GNU/Linux in general as we have finally had a great alternative to OpenGL.

If possible, the movement of mesa-vulkan-drivers out of Universe as well would be a HUGE benefit. Especially if mesa-vulkan-drivers is installed by default in Ubuntu 18.04 bringing in full support for Vulkan. This is helpful especially since Ubuntu 18.04 is a long term support release.

[Teg (tegskywalker)]

Ubuntu 18.04 (Bionic)

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Hans Joachim Desserud (hjd)]

Thanks for taking your time to report this issue and help making Ubuntu better.

When adding packages to the main repo, there's a template and set of information which is usually listed. Could you take a look at https://wiki.ubuntu.com/MainInclusionProcess and subscribe https://launchpad.net/~ubuntu-mir once the description is updated? :)

[Timo Aaltonen (tjaalton)]

vulkan is already "available", and if the drivers get installed by default then vulkan would move too. In any case, no point in having two bugs about the same thing.

[Timo Aaltonen (tjaalton)]

we'll add mesa-vulkan-drivers to ubuntu-desktop depends (bug 1765800), which means libvulkan1 will need to be moved to main

[Didier Roche-Tolomelli (didrocks)]

* "upstream split the source in four, old src:vulkan is going away and src:vulkan-loader will be the one to be promoted once it's accepted from NEW."
-> it seems as of now (like 9 months later), the source is still named vulkan, but I see vulkan-loader in NEW. The content of the package is exactly the same, or should the MIR review be redone. What's the split cover exactly?

* we are bundling GLSLANG and SPIR-V in the source package. I think that the MIR description (as you are aware of it, having written some wrapper to get theme) should have mentioned it. Is it something you discussed with the security team, as we already have glslang and spir-v as part of the archive (even if I understand the benefits of vendoring, but here, you aren't even sure to use the same vendored version than upstream as they don't give you specific sha to build against)?

* debian/patches/*
-> would be good for them to follow DEP3, so that they all have descriptions and rationale (some are just diff dumping a file with no explanations, or changing the build system)

* debian/rules:
-DBUILD_TESTS=OFF -> any chance to enable the tests for this package?

* In the bug list, looking at NEW ones, there is only one crashers. The other ones can be closed, do you mind doing a cleanswap on the NEW bugs?

* debian/copyright:
the external vendored dependencies aren't listed, like external/glslang/OGLCompilersDLL/InitializeDll.cpp: BSD (3 clause)

Nitpick:
- you are using debian format source3, quilt. You thus don't need to add a quilt build-dep.
- debian/rules is using --with quilt. Same, than above, shouldn't be needed for source format 3, quilt.

[Timo Aaltonen (tjaalton)]

You should probably review the packages from NEW, bundled glslang and spirv got dropped and archive versions are used now (spirv-tools still in NEW).

old src:vulkan got split into
vulkan-headers
vulkan-loader
vulkan-tools
vulkan-validationlayers

vulkan-headers is in the archive, but I've actually merged it back into vulkan-loader since the split makes no sense. So once the new packages are in src:vulkan-headers can be removed.

The crasher was about Mir backend, doubt it's an issue nowadays with wayland client support in Mir.

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Didier Roche-Tolomelli (didrocks)]

I would like the rationale to be updated to reflect what will be promoted from vulkan-loader, please.

[Didier Roche-Tolomelli (didrocks)]

Setting as incomplete until I hear back.

[Timo Aaltonen (tjaalton)]

vulkan-loader builds two packages, libvulkan1 and libvulkan-dev. Mesa build-depens on libvulkan-dev, but since build-depends from universe are fine then it should be enough to promote libvulkan1

and that's also what will want to migrate once ubuntu-desktop depends on mesa-vulkan-drivers

about the comments in #6;

- there's only one patch, to fix pkgconfig, which is self-explanatory
- I'm looking into building with tests enabled, but IIRC running them by hand shows that ~10% pass, which means there's something systematically wrong which I haven't wrapped my head around yet
- source-format/quilt; yeah that's probably a leftover of the old packaging, will fix it

[Launchpad Janitor (janitor)]

[Expired for vulkan-loader (Ubuntu) because there has been no activity for 60 days.]

[Timo Aaltonen (tjaalton)]

reopening

[Timo Aaltonen (tjaalton)]

So I had a look at the tests, and they actually need a hardware driver to work. There is no CPU-based driver available (yet), but I'll enable them in the packaging with a note that most of them are expected to fail.

I also dropped quilt from control/rules as it was leftover cruft from old packaging.

These are both in 1.1.101-2

[Mathieu Trudel-Lapierre (cyphermox)]

Looks generally good to me, but this is a very large and complex project. I've tried to review the code as much as I could, but I would feel better if there were more eyes on it. Assigning to ~ubuntu-security ... I'm not requesting a full security code review, but leaving it to the Security Team to ack or decide if it really should need a more in-depth code review.

[Alex Murray (alexmurray)]

I reviewed vulkan-loader version 1.1.101.0-2_amd64 as checked into
disco. This shouldn't be considered a full security audit but rather a
quick check of maintainability.

- No CVE history in our database
- vulkan-loader provides support for loading the main vulkan library,
  handling layer and driver management including multi-gpu support to
  dispatch API calls to the correct driver and layer.
- Depends: debhelper, cmake, googletest, libwayland-dev, libx11-dev,
  libxcb1-dev, libxrandr-dev, pkg-config, python3
- Does not itself do networking
- No cryptography
- Does not daemonize
- No pre/post inst/rm
- No init scripts
- No dbus services
- No setuid files
- No binaries in the PATH
- No sudo fragments
- No udev rules
- A test suite is run during the build (as noted in the log, 23 of the
  tests fail due to missing vulkan driver but as this is expected this
  is not a concern)
- No cron jobs
- 3 warnings in build logs about memory allocation functions which
  declare as returning void * but are used for functions which expect an
  unsigned long * return value - these can safely be ignored
- No cppcheck warnings

- No subprocesses spawned
- Memory management is very careful in general, however I noticed that
  the loader allocates a buffer on stack for reading in ICD JSON
  descriptions - this uses the length of the JSON file as the length of
  the buffer to allocate and since these files can be user controlled it
  could be relatively easily exploited by dropping a very large JSON
  file to overrun the stack (since uses alloca() internally which has
  undefined behaviour if stack is overflown) - this might be worth
  investigating further but is really only a denial of service issue so
  not a high priority and no chance of privilege escalation etc
- Otherwise most memory management is quite careful, allocation return
  values are checked for failure, buffer lengths are checked, string
  lengths are checked and handled correctly etc.
- Does not itself do file IO beyond reading JSON as described above
- Logging is careful
- Uses the following environment variables:
  - VK_LOADER_DISABLE_INST_EXT_FILTER
  - VK_LOADER_DEBUG
  - XDG_CONFIG_DIRS
  - XDG_DATA_DIRS
  - XDG_DATA_HOME
  - HOME
- No privileged code sections
- No privileged functions
- No networking
- No temp files
- No WebKit
- No PolKit

Security team ACK for promoting vulkan-loader to main for disco.

[Didier Roche-Tolomelli (didrocks)]

$ ./change-override -c main -t vulkan-loader
Override component to main
vulkan-loader 1.1.101.0-2 in disco: universe/libs -> main
Override [y|N]? y
1 publication overridden.
$ ./change-override -c main libvulkan1
Override component to main
libvulkan1 1.1.101.0-2 in disco amd64: universe/libs/optional/100% -> main
libvulkan1 1.1.101.0-2 in disco arm64: universe/libs/optional/100% -> main
libvulkan1 1.1.101.0-2 in disco armhf: universe/libs/optional/100% -> main
libvulkan1 1.1.101.0-2 in disco i386: universe/libs/optional/100% -> main
libvulkan1 1.1.101.0-2 in disco ppc64el: universe/libs/optional/100% -> main
libvulkan1 1.1.101.0-2 in disco s390x: universe/libs/optional/100% -> main
Override [y|N]? y
6 publications overridden.