search

NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.8k stars 13.91k forks source link

Add a default search path to vulkan-loader #22990

Closed abbradar closed 7 years ago

abbradar commented 7 years ago

Issue description

See https://github.com/NixOS/nixpkgs/pull/22284 for previous discussion. Currently loader uses XDG paths for discovering Vulkan drivers but this shouldn't work with SUID binaries like in related VirtualBox issue because one can inject malicious library otherwise. Adding mesa.driverLink as a fallback path should fix this -- what do you think?

cc @Ralith @corngood

Ralith commented 7 years ago

Per my comments on https://github.com/KhronosGroup/Vulkan-LoaderAndValidationLayers/issues/1492 I do not believe the XDG path search constitutes a security issue, though I do not vouch for the loader as a whole. However, I agree that we should make the loader able to find system-wide drivers (and layers) when ran as SUID/SGID. This could be accomplished by patching the loader to search /run/opengl-driver{,-32}/share, which seems to indeed be the value of mesa.driverLink, when the XDG env vars cannot be read. Unless anyone has any objections I'll draft a PR implementing this.

Ralith commented 7 years ago

PR's up. Note that this patch only affects behavior when the XDG vars are empty, unset, or inaccessible for security reasons; the XDG vars still solely determine behavior when possible. This isn't exactly the approach you seemed to be advocating in #22284, but AFAICT it addresses the concrete issues effectively; at this point unconditionally adding mesa.driverLink to the search path would only represent a decrease in flexibility.

abbradar commented 7 years ago

I actually agree with the approach -- I'm entirely okay with not using fallback by default as long as it will be used as the last resort. Thank you for the patch (and extra for your efforts in upstreaming it), I'll test it later!