Comment 7 for bug 1946359

Revision history for this message
Rodrigo Figueiredo Zaiden (rodrigo-zaiden) wrote :

I reviewed vulkan 1.1.70+dfsg1-1ubuntu0.18.04.1 as checked into bionic.
This shouldn't be considered a full audit but rather a quick gauge of
maintainability.

vulkan is an API to control GPUs with many attributes, mainly: driver loaders,
validation layers and tools. This source package existed up until bionic and
was later split in 3 other packages: vulkan-loader, vulkan-tools and
vulkan-validationlayers. The MIR is targeting vulkan-utils package that in
later releases is part of the vulkan-tools source package.

My main concern is that upstream does not maintain this code[1] anymore, it was
moved to other repos[2][3][4][5] and the code does not match, that is, the
backport is not trivial and more than that, the code has changed too much and
many fixes can't be backported as far as I could dig into.

The most concerning issues would be (the highs from Coverity, excluding
external/):
- An out-of-bounds in loader/loader.c:3403, it was fixed in vulkan-loader but
the backport is quite a big change[6], increasing the chance of regressions.

- A memory overlap issue in loader/ that still exists in the newer repos, and
I've reported to upstream to get their opinion:
https://github.com/KhronosGroup/Vulkan-Loader/issues/988

- Two use-after-free issues in layers/vk_layer_logging.h that I couldn't find
the corresponding fix in vulkan-layer new repo[4].

I understand that the loader/ and layers/, the main points of my criticism were
moved to vulkan-loader, that is not in the MIR for newer releases (it is
vulkan-tools), but for bionic I believe we can't skip checking it since we would
be promoting vulkan-utils package. So, I would suggest this MIR to be rejected.
From a security point of view, this code is not maintainable.

Security team NACK for promoting vulkan to main: the code maintainability is
not sustainable.

[1] https://github.com/KhronosGroup/Vulkan-LoaderAndValidationLayers
[2] https://github.com/KhronosGroup/Vulkan-Headers
[3] https://github.com/KhronosGroup/Vulkan-Loader
[4] https://github.com/KhronosGroup/Vulkan-ValidationLayers
[5] https://github.com/KhronosGroup/Vulkan-Tools
[6] https://github.com/KhronosGroup/Vulkan-Loader/commit/40761b0913d3fbc4b618d4837ae0d06cc7c47508