Hi On Fri, Aug 29, 2014 at 2:57 PM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote: > 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. 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 -- 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