GNOME Bugzilla – Bug 781034
monitor-manager-kms may set incorrect libdrm event context version
Last modified: 2019-01-02 14:47:20 UTC
DRM_EVENT_CONTEXT_VERSION is the latest context version supported by whatever version of libdrm is present. Mutter was blindly asserting it supported whatever version that may be, even if it actually didn't. With libdrm 2.4.78, setting a higher context version than 2 will attempt to call the page_flip_handler2 vfunc if it was non-NULL, which being a random chunk of stack memory, it might well have been. Set the version as 2, which should be bumped only with the appropriate version checks.
Created attachment 349476 [details] [review] monitor-manager-kms: Use correct DRM event context version DRM_EVENT_CONTEXT_VERSION is the latest context version supported by whatever version of libdrm is present. Mutter was blindly asserting it supported whatever version that may be, even if it actually didn't. With libdrm 2.4.78, setting a higher context version than 2 will attempt to call the page_flip_handler2 vfunc if it was non-NULL, which being a random chunk of stack memory, it might well have been. Set the version as 2, which should be bumped only with the appropriate version checks.
Please merge when you can; I don't have commit access. This is also a good candidate for stable branches.
Can I say I don't understand why this patch is needed? If mutter is compiled against old libdrm and run against old libdrm, the version will be the old one and libdrm will not touch page_flip_handler2. If mutter is compiled against old libdrm, and run against new libdrm, the version should still be the old one (it's a constant), so the new libdrm should not look at page_flip_handler2 (because it will be garbage). If mutter is compiled against new libdrm, the version will be new but at the same time the struct will be one pointer larger, meaning that the memset will zero out the pointer and libdrm should not call page_flip_handler2. When is this causing issues?
I'd argue the patch is correct, but it indeed seems more like a cleanup than anything needed, given Giovannis reasoning above.
You're right, in that the memset helps. But equally, a context version bump could change semantics and behaviour entirely, and you'd end up being broken by it.
FWIW, the patch does still apply.
This wasn't landed for some reason. I reworked the patch (the affected function had moved to another file), and push.d