Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 91eefc05 authored by Daniel Vetter's avatar Daniel Vetter
Browse files

drm: Tighten locking in drm_mode_getconnector 



- Modeset state needs mode_config->connection mutex, that covers
  figuring out the encoder, and reading properties (since in the
  atomic case those need to look at connector->state).

- Don't hold any locks for stuff that's invariant (i.e. possible
  connectors).

- Same for connector lookup and unref, those don't need any locks.

- And finally the probe stuff is only protected by mode_config->mutex.

While at it updated the kerneldoc for these fields in drm_connector
and add docs explaining what's protected by which locks.

Reviewed-by: default avatarSean Paul <seanpaul@chromium.org>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20161213230814.19598-10-daniel.vetter@ffwll.ch
parent e73ab00e

drivers/gpu/drm/drm_connector.c

+44 −48
Original line number Diff line number Diff line
@@ -1163,43 +1163,65 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, 


	memset(&u_mode, 0, sizeof(struct drm_mode_modeinfo));



	mutex_lock(&dev->mode_config.mutex);



	connector = drm_connector_lookup(dev, out_resp->connector_id);

	if (!connector) {

		ret = -ENOENT;

		goto out_unlock;

	}

	if (!connector)

		return -ENOENT;



	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);

	encoder = drm_connector_get_encoder(connector);

	if (encoder)

		out_resp->encoder_id = encoder->base.id;

	else

		out_resp->encoder_id = 0;



	ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic,

			(uint32_t __user *)(unsigned long)(out_resp->props_ptr),

			(uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr),

			&out_resp->count_props);

	drm_modeset_unlock(&dev->mode_config.connection_mutex);

	if (ret)

		goto out_unref;



	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)

		if (connector->encoder_ids[i] != 0)

			encoders_count++;



	if ((out_resp->count_encoders >= encoders_count) && encoders_count) {

		copied = 0;

		encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr);

		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {

			if (connector->encoder_ids[i] != 0) {

				if (put_user(connector->encoder_ids[i],

					     encoder_ptr + copied)) {

					ret = -EFAULT;

					goto out_unref;

				}

				copied++;

			}

		}

	}

	out_resp->count_encoders = encoders_count;



	out_resp->connector_id = connector->base.id;

	out_resp->connector_type = connector->connector_type;

	out_resp->connector_type_id = connector->connector_type_id;



	mutex_lock(&dev->mode_config.mutex);

	if (out_resp->count_modes == 0) {

		connector->funcs->fill_modes(connector,

					     dev->mode_config.max_width,

					     dev->mode_config.max_height);

	}



	/* delayed so we get modes regardless of pre-fill_modes state */

	list_for_each_entry(mode, &connector->modes, head)

		if (drm_mode_expose_to_userspace(mode, file_priv))

			mode_count++;



	out_resp->connector_id = connector->base.id;

	out_resp->connector_type = connector->connector_type;

	out_resp->connector_type_id = connector->connector_type_id;

	out_resp->mm_width = connector->display_info.width_mm;

	out_resp->mm_height = connector->display_info.height_mm;

	out_resp->subpixel = connector->display_info.subpixel_order;

	out_resp->connection = connector->status;



	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);

	encoder = drm_connector_get_encoder(connector);

	if (encoder)

		out_resp->encoder_id = encoder->base.id;

	else

		out_resp->encoder_id = 0;

	/* delayed so we get modes regardless of pre-fill_modes state */

	list_for_each_entry(mode, &connector->modes, head)

		if (drm_mode_expose_to_userspace(mode, file_priv))

			mode_count++;



	/*

	 * This ioctl is called twice, once to determine how much space is
@@ -1222,36 +1244,10 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, 
		}

	}

	out_resp->count_modes = mode_count;



	ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic,

			(uint32_t __user *)(unsigned long)(out_resp->props_ptr),

			(uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr),

			&out_resp->count_props);

	if (ret)

		goto out;



	if ((out_resp->count_encoders >= encoders_count) && encoders_count) {

		copied = 0;

		encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr);

		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {

			if (connector->encoder_ids[i] != 0) {

				if (put_user(connector->encoder_ids[i],

					     encoder_ptr + copied)) {

					ret = -EFAULT;

					goto out;

				}

				copied++;

			}

		}

	}

	out_resp->count_encoders = encoders_count;



out:

	drm_modeset_unlock(&dev->mode_config.connection_mutex);



	drm_connector_unreference(connector);

out_unlock:

	mutex_unlock(&dev->mode_config.mutex);

out_unref:

	drm_connector_unreference(connector);



	return ret;

}

include/drm/drm_connector.h

+19 −4
Original line number Diff line number Diff line
@@ -563,9 +563,6 @@ struct drm_cmdline_mode { 
 * @interlace_allowed: can this connector handle interlaced modes?

 * @doublescan_allowed: can this connector handle doublescan?

 * @stereo_allowed: can this connector handle stereo modes?

 * @modes: modes available on this connector (from fill_modes() + user)

 * @status: one of the drm_connector_status enums (connected, not, or unknown)

 * @probed_modes: list of modes derived directly from the display

 * @funcs: connector control functions

 * @edid_blob_ptr: DRM property containing EDID if present

 * @properties: property tracking for this connector
@@ -635,11 +632,27 @@ struct drm_connector { 
	 * Protected by @mutex.

	 */

	bool registered;



	/**

	 * @modes:

	 * Modes available on this connector (from fill_modes() + user).

	 * Protected by dev->mode_config.mutex.

	 */

	struct list_head modes; /* list of modes on this connector */



	/**

	 * @status:

	 * One of the drm_connector_status enums (connected, not, or unknown).

	 * Protected by dev->mode_config.mutex.

	 */

	enum drm_connector_status status;



	/* these are modes added by probing with DDC or the BIOS */

	/**

	 * @probed_modes:

	 * These are modes added by probing with DDC or the BIOS, before

	 * filtering is applied. Used by the probe helpers.Protected by

	 * dev->mode_config.mutex.

	 */

	struct list_head probed_modes;



	/**
@@ -648,6 +661,8 @@ struct drm_connector { 
	 * flat panels in embedded systems, the driver should initialize the

	 * display_info.width_mm and display_info.height_mm fields with the

	 * physical size of the display.

	 *

	 * Protected by dev->mode_config.mutex.

	 */

	struct drm_display_info display_info;

	const struct drm_connector_funcs *funcs;