Critical: Illegal type-casting of GBM buffers in clients
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Mir |
Fix Released
|
High
|
Alexandros Frantzis | ||
mir (Ubuntu) |
Fix Released
|
High
|
Unassigned |
Bug Description
Found as part of bringing up Mir on vmwgfx.
It seems like Mir is using dma buffers in an illegal way:
1) Mir creates a GBM bufffer.
2) Mir uses Prime to export a dma_buf handle which it shares with its
clients.
3) The client imports the dma_buf handle and uses drm to turn it into a
drm buffer handle.
4) The buffer handle is typecast to a "dumb" buffer handle, and then
mmap'ed. in struct GBMMemoryRegion : mcl::MemoryRegion.
It's illegal to typecast a GBM buffer to a dumb buffer in this way. It
accidently happens to work on the major driver because deep inside, both
a GBM buffer and a dumb buffer is represented by a GEM buffer object.
With vmwgfx that's not the case either for a GBM buffer or a dumb
buffer, and they are different objects.
In fact, currently the only way to mmap() a GBM buffer (unless it's a
cursor buffer) is to export a dma_buf and use it's mmap() operation. But
that is not implemented by any of the major drivers since it's not
really desired. The reason is that a GBM buffer is completely opaque and
may not even reside in mappable memory. Hence any attempt to map it may
result in coherence issues and, in some cases, extremely costly
operations. This results in awkward driver code that attempts to guess
the usage-patterns of applications that mix mmap'ed cpu-access and
accelerated access to the same object.
The correct way to do this is to have the client import the buffer to
the appropriate API, perhaps as an EGLImage, and then use pixel write operations
or texSubImage operations on that image.
This makes the client or user think twice about what data is actually
being transferred and in what direction, rather than having the driver
guess and assume the worst case.
Related branches
- Alan Griffiths: Approve
- Daniel van Vugt: Approve
- Kevin DuBois (community): Approve
- PS Jenkins bot (community): Approve (continuous-integration)
-
Diff: 895 lines (+154/-288)14 files modifiedsrc/client/aging_buffer.h (+0/-1)
src/client/client_buffer.h (+6/-0)
src/client/gbm/buffer_file_ops.h (+11/-13)
src/client/gbm/gbm_client_buffer.cpp (+20/-65)
src/client/gbm/gbm_client_buffer.h (+7/-9)
src/client/gbm/gbm_client_buffer_factory.cpp (+3/-3)
src/client/gbm/gbm_client_buffer_factory.h (+3/-3)
src/client/gbm/gbm_client_platform.cpp (+13/-40)
src/client/gbm/gbm_client_platform.h (+3/-3)
src/server/graphics/gbm/buffer_allocator.cpp (+47/-8)
src/server/graphics/gbm/buffer_allocator.h (+5/-1)
tests/unit-tests/client/gbm/mock_drm_fd_handler.h (+0/-46)
tests/unit-tests/client/gbm/test_gbm_client_buffer.cpp (+35/-92)
tests/unit-tests/graphics/gbm/test_gbm_buffer_allocator.cpp (+1/-4)
tags: | added: vmware |
Changed in mir: | |
importance: | Undecided → High |
status: | New → Triaged |
assignee: | nobody → Alexandros Frantzis (afrantzis) |
Changed in mir: | |
milestone: | none → 0.1.3 |
Changed in mir (Ubuntu): | |
status: | New → Triaged |
importance: | Undecided → High |
Changed in mir: | |
status: | Fix Committed → Fix Released |
Changed in mir (Ubuntu): | |
status: | Triaged → Fix Released |
This was fixed in lp:mir/devel revision 1261, by using shm buffers for the "software" buffer case.