I want to use this short article to point out a big problem in code that uses libdrm. This, or a variant of this, is lurking in a shocking amount of software:
drmEventContext evctx;evctx.version = DRM_EVENT_CONTEXT_VERSION; evctx.page_flip_handler = pfhdlr; evctx.vblank_handler = vbnkhdr;
Stop doing this, it has always been a crash waiting to happen.
drmEventContext is part of libdrm’s ABI and there are strict promises that the structure won’t be randomly re-ordered. However, it may grow as the need arises when new versions of libdrm offer functionality which didn’t exist before.
libdrm 2.4.78 introduces another page flip handler, adding a new function pointer harmlessly at the end of the structure page_flip_handler2. It also bumps DRM_EVENT_CONTEXT_VERSION from 2 to 3, as that’s the current version of the structure in the header file.
As a result, the code above crashes… sometimes. What went wrong?
The Proper Way to Use drmEventContext
DrmEventContext.version is intended to tell the library what version of the event context functionality an application understands. The DRM_EVENT_CONTEXT_VERSION macro indicates the highest version the installed libdrm you compiled against understands.
The new page_flip_handler2 function will only be used if the calling program sets the version to 3, declaring it knows about page_flip_handler2 and has set it correctly. The snippet declared its version to always be the most recent version of the structure, yet left the new page_flip_handler2 pointer uninitialized, meaning it might be non-NULL and might be called as a function: potentially causing the program to crash.
EFL had a variant of this:
drmEventContext ctx; memset(&ctx, 0, sizeof(ctx)); ...
This, at a glance, looks safe since the new pointer is always initialized to NULL. But, this is potentially broken too as the context version could, in theory, change something behavioral. So we’ve updated our code to be future proof, and any code you have that uses libdrm you should too! :)
The fix is trivially different:
drmEventContext evctx;evctx.version = 2; evctx.page_flip_handler = pfhdlr; evctx.vblank_handler = vbnkhdr;
Unless, of course, your code knows about page_flip_handler2 and sets it correctly, then go ahead and set version to 3.
(And we’re all left wondering why that macro exists at all…)