From 2ee39452fa2fff1e8edb954ccb7e0daee9646557 Mon Sep 17 00:00:00 2001 From: Dave Airlie Date: Thu, 24 Jul 2014 11:53:45 +1000 Subject: [PATCH] drm: close race in connector registration (v2) Daniel pointed out with hotplug that userspace could be trying to oops us as root for lols, and that to be correct we shouldn't register the object with the idr before we have fully set the connector object up. His proposed solution was a lot more life changing, this seemed like a simpler proposition to me, get the connector object id from the idr, but don't register the object until the drm_connector_register callback. The open question is whether the drm_mode_object_register needs a bigger lock than just the idr one, but I can't see why it would, but I can be locking challenged. v2: fix bool noreg into sane - add comment. Reviewed-by: Daniel Vetter Reviewed-by: David Herrmann Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_crtc.c | 48 +++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 1ccf5cb9aa86..c3a5a8f0825b 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -361,6 +361,32 @@ const char *drm_get_format_name(uint32_t format) } EXPORT_SYMBOL(drm_get_format_name); +/* + * Internal function to assign a slot in the object idr and optionally + * register the object into the idr. + */ +static int drm_mode_object_get_reg(struct drm_device *dev, + struct drm_mode_object *obj, + uint32_t obj_type, + bool register_obj) +{ + int ret; + + mutex_lock(&dev->mode_config.idr_mutex); + ret = idr_alloc(&dev->mode_config.crtc_idr, register_obj ? obj : NULL, 1, 0, GFP_KERNEL); + if (ret >= 0) { + /* + * Set up the object linking under the protection of the idr + * lock so that other users can't see inconsistent state. + */ + obj->id = ret; + obj->type = obj_type; + } + mutex_unlock(&dev->mode_config.idr_mutex); + + return ret < 0 ? ret : 0; +} + /** * drm_mode_object_get - allocate a new modeset identifier * @dev: DRM device @@ -379,21 +405,15 @@ EXPORT_SYMBOL(drm_get_format_name); int drm_mode_object_get(struct drm_device *dev, struct drm_mode_object *obj, uint32_t obj_type) { - int ret; + return drm_mode_object_get_reg(dev, obj, obj_type, true); +} +static void drm_mode_object_register(struct drm_device *dev, + struct drm_mode_object *obj) +{ mutex_lock(&dev->mode_config.idr_mutex); - ret = idr_alloc(&dev->mode_config.crtc_idr, obj, 1, 0, GFP_KERNEL); - if (ret >= 0) { - /* - * Set up the object linking under the protection of the idr - * lock so that other users can't see inconsistent state. - */ - obj->id = ret; - obj->type = obj_type; - } + idr_replace(&dev->mode_config.crtc_idr, obj, obj->id); mutex_unlock(&dev->mode_config.idr_mutex); - - return ret < 0 ? ret : 0; } /** @@ -849,7 +869,7 @@ int drm_connector_init(struct drm_device *dev, drm_modeset_lock_all(dev); - ret = drm_mode_object_get(dev, &connector->base, DRM_MODE_OBJECT_CONNECTOR); + ret = drm_mode_object_get_reg(dev, &connector->base, DRM_MODE_OBJECT_CONNECTOR, false); if (ret) goto out_unlock; @@ -942,6 +962,8 @@ int drm_connector_register(struct drm_connector *connector) { int ret; + drm_mode_object_register(connector->dev, &connector->base); + ret = drm_sysfs_connector_add(connector); if (ret) return ret; -- 2.20.1