On 11/30/16 00:06, Laurent Pinchart wrote: >> > /** >> > * drm_bridge_remove - remove the given bridge from the global bridge list >> > @@ -114,6 +118,9 @@ int drm_bridge_attach(struct drm_device *dev, struct >> > drm_bridge *bridge) if (bridge->dev) >> > return -EBUSY; >> > >> > + if (!try_module_get(bridge->module)) >> > + return -ENOENT; > Isn't this still racy ? What happens if the module is removed right before > this call ? Won't the bridge object be freed, and this code then try to call > try_module_get() on freed memory ? > > To fix this properly I think we need to make the bridge object refcounted, > with a release callback to signal to the bridge driver that memory can be > freed. The refcount should be increased in of_drm_find_bridge(), and decreased > in a new drm_bridge_put() function (the "fun" part will be to update drivers > to call that :-S). > I think another option would be to find and attach the bridge object atomically by holding the bridge_lock until the try_module_get() has succeeded. AFAIU, if the module unload is triggered while holding the bridge_lock but before the try_module_get() call, then try_module_get() would return false and the attach call will return failure. > The module refcount still needs to be increased in drm_bridge_attach() like > you do here, but you'll need to protect it with bridge_lock to avoid a race > between try_module_get() and drm_bridge_remove(). > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
