search

KhronosGroup / Vulkan-Docs

The Vulkan API Specification and related tools
Other
2.82k stars 469 forks source link

Clarify that imported buffers cannot cause data races #2408

Open DemiMarie opened 3 months ago

DemiMarie commented 3 months ago

The Vulkan specification requires that applications not cause data races:

Applications must ensure that no data races occur during the execution of their application.

However, this requirement cannot be satisfied for imported buffers, because the exporting process may modify the memory at any time. While a correct non-malicious exporter will not do so, a data race caused by a buggy or malicious exporter must not be able to harm the importing application. Due to memory bandwidth limitations, I suspect it is unreasonable to expect importing programs to perform a defensive copy using one of the copy commands.

cubanismo commented 3 months ago

This is beyond the level of security provided by the current external memory extensions. The external memory extensions guarantee only that the applications won't be able to clobber each-others' non-shared resources or cause the other application to terminate IIRC. Beyond that, some level of mutual trust is indeed assumed. If you have an environment that requires stronger protections and can provide them, you could define additional extensions defining the additional guarantees there.

DemiMarie commented 3 months ago

From what I understand, Wayland compositors are expected to not need to have any trust in their clients, and implementations of the dmabuf extension are expected to ensure this. Furthermore, Wayland compositors do not actually perform any copies of imported buffers. This means that the guarantees provided by this extension are weaker than both what is assumed by its users and what is expected from its implementors. In particular, data races are (to the best of my understanding) undefined behavior, which means anything can happen. This means that the current specification is inconsistent.

While it would be possible to add a new extension to codify this, I think it would be more useful to update the current spec to match what is both assumed and (to the best of my knowledge) provided.

I’m CCing @marmarek (from Qubes OS), @emersion (wlroots lead maintainer), @gfxstrand (Linux graphics stack developer), and @robclark (ChromeOS).