Hi On Fri, Aug 29, 2014 at 2:51 PM, Daniel Vetter <daniel@xxxxxxxx> 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@xxxxxxxxxxxxxxx> # 3.16+ >> Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx> > > 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. Thanks David -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html