Hi
On Fri, Aug 29, 2014 at 2:57 PM, David Herrmann dh.herrmann@gmail.com wrote:
Hi
On Fri, Aug 29, 2014 at 2:51 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Fri, Aug 29, 2014 at 02:01:01PM +0200, David Herrmann wrote:
With MST, we now have connector hotplugging, yey! Pretty easy to use in user-space, but introduces some nasty races:
- If a connector is removed and added again while a compositor is in background, it will get the same ID again. If the compositor wakes up, it cannot know whether its the same connector or a new one, thus they must re-read EDID information, etc.
- possible_clones, possible_crtcs, etc. depend on indices, not IDs. So if an object is removed and a new one is added, those bitmasks are invalid and must be refreshed. This currently does not affect connectors, but only crtcs and encoders, but it's only a matter of time when this will change.
The easiest way to protect against this, is to not recylce modeset IDs. Instead, always allocate a new, higher ID. All ioctls that affect modeset objects, take IDs. Thus, during hotplug races, those ioctls will simply fail if invalid IDs are passed. They will no longer silently run on a newly hotplugged object.
Furthermore, hotplug-races during state sync can now be easily detected. A call to GET_RESOURCES returns a list of available IDs atomically. User-space can now start fetching all those objects via GET_* ioctls. If any of those fails, they know that the given object was unplugged. Thus, the "possible_*" bit-fields are invalidated. User-space can now decide whether to restart the sync all over or wait for the 'change' uevent that is sent on modeset object modifications (or, well, is supposed to be sent and probably will be at some point).
With this in place, modeset object hotplugging should work fine for all modeset objects in the KMS API.
CC'ing stable so we can rely on all kernels with MST to not recycle IDs.
Cc: stable@vger.kernel.org # 3.16+ Signed-off-by: David Herrmann dh.herrmann@gmail.com
So userspace just needs to cycle through piles of framebuffer objects to make bad things happen? Doesn't sound like a terribly solid plan.
IDs will still get recycled, but only once all IDs got used. So this is "safe".
Sure, user-space can create 2 billion framebuffers and destroy them again, thus causing the ID range to overflow and recycle old IDs. Not sure how fast you can run 2 billion syscalls.. If that's a real issue, I'd vote for using the high-range for user-space managed objects, low-range for kernel-space managed objects ([1...INT_MAX] and [INT_MAX+1...UINT_MAX] or so).
I guess we could save this by doing normal id allocations for fbs and monotonically increasing allocations for all other objects.
This doesn't work. A connector with ID #n might get unplugged and another process created a new FB, which will then get ID #n. Sure, I doubt there's a real conflict as ioctls check the type, but it still sounds really ugly to me.
On a second thought: maybe your idea isn't as bad as I thought. I mean, everyone must do type-checking when looking up mode-objects, so it seems safe to rely on that.
Thanks David