Commit f52b69f1 authored by Daniel Vetter's avatar Daniel Vetter Committed by Dave Airlie
Browse files

drm/atomic: Don't overrun the connector array when hotplugging 



Yet another fallout from not considering DP MST hotplug. With the
previous patches we have stable indices, but it might still happen
that a connector gets added between when we allocate the array and
when we actually add a connector. Especially when we back off due to
ww mutex contention or similar issues.

So store the sizes of the arrays in struct drm_atomic_state and double
check them. We don't really care about races except that we want to
use a consistent value, so ACCESS_ONCE is all we need. And if we
indeed notice that we'd overrun the array then just give up and
restart the entire ioctl.

Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Reviewed-by: default avatarRob Clark <robdclark@gmail.com>
Signed-off-by: default avatarDave Airlie <airlied@redhat.com>
parent 6f75cea6

drivers/gpu/drm/drm_atomic.c

+21 −5
Original line number Diff line number Diff line
@@ -56,6 +56,8 @@ drm_atomic_state_alloc(struct drm_device *dev) 
	if (!state)

		return NULL;



	state->num_connector = ACCESS_ONCE(dev->mode_config.num_connector);



	state->crtcs = kcalloc(dev->mode_config.num_crtc,

			       sizeof(*state->crtcs), GFP_KERNEL);

	if (!state->crtcs)
@@ -72,12 +74,12 @@ drm_atomic_state_alloc(struct drm_device *dev) 
				      sizeof(*state->plane_states), GFP_KERNEL);

	if (!state->plane_states)

		goto fail;

	state->connectors = kcalloc(dev->mode_config.num_connector,

	state->connectors = kcalloc(state->num_connector,

				    sizeof(*state->connectors),

				    GFP_KERNEL);

	if (!state->connectors)

		goto fail;

	state->connector_states = kcalloc(dev->mode_config.num_connector,

	state->connector_states = kcalloc(state->num_connector,

					  sizeof(*state->connector_states),

					  GFP_KERNEL);

	if (!state->connector_states)
@@ -117,7 +119,7 @@ void drm_atomic_state_clear(struct drm_atomic_state *state) 


	DRM_DEBUG_KMS("Clearing atomic state %p\n", state);



	for (i = 0; i < config->num_connector; i++) {

	for (i = 0; i < state->num_connector; i++) {

		struct drm_connector *connector = state->connectors[i];



		if (!connector)
@@ -304,6 +306,21 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state, 


	index = drm_connector_index(connector);



	/*

	 * Construction of atomic state updates can race with a connector

	 * hot-add which might overflow. In this case flip the table and just

	 * restart the entire ioctl - no one is fast enough to livelock a cpu

	 * with physical hotplug events anyway.

	 *

	 * Note that we only grab the indexes once we have the right lock to

	 * prevent hotplug/unplugging of connectors. So removal is no problem,

	 * at most the array is a bit too large.

	 */

	if (index >= state->num_connector) {

		DRM_DEBUG_KMS("Hot-added connector would overflow state array, restarting\n");

		return -EAGAIN;

	}



	if (state->connector_states[index])

		return state->connector_states[index];
@@ -499,10 +516,9 @@ int 
drm_atomic_connectors_for_crtc(struct drm_atomic_state *state,

			       struct drm_crtc *crtc)

{

	int nconnectors = state->dev->mode_config.num_connector;

	int i, num_connected_connectors = 0;



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

	for (i = 0; i < state->num_connector; i++) {

		struct drm_connector_state *conn_state;



		conn_state = state->connector_states[i];

drivers/gpu/drm/drm_atomic_helper.c

+8 −15
Original line number Diff line number Diff line
@@ -249,7 +249,6 @@ static int 
mode_fixup(struct drm_atomic_state *state)

{

	int ncrtcs = state->dev->mode_config.num_crtc;

	int nconnectors = state->dev->mode_config.num_connector;

	struct drm_crtc_state *crtc_state;

	struct drm_connector_state *conn_state;

	int i;
@@ -264,7 +263,7 @@ mode_fixup(struct drm_atomic_state *state) 
		drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode);

	}



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

	for (i = 0; i < state->num_connector; i++) {

		struct drm_encoder_helper_funcs *funcs;

		struct drm_encoder *encoder;
@@ -336,7 +335,6 @@ drm_atomic_helper_check_prepare(struct drm_device *dev, 
				struct drm_atomic_state *state)

{

	int ncrtcs = dev->mode_config.num_crtc;

	int nconnectors = dev->mode_config.num_connector;

	struct drm_crtc *crtc;

	struct drm_crtc_state *crtc_state;

	int i, ret;
@@ -361,7 +359,7 @@ drm_atomic_helper_check_prepare(struct drm_device *dev, 
		}

	}



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

	for (i = 0; i < state->num_connector; i++) {

		/*

		 * This only sets crtc->mode_changed for routing changes,

		 * drivers must set crtc->mode_changed themselves when connector
@@ -485,10 +483,9 @@ static void 
disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)

{

	int ncrtcs = old_state->dev->mode_config.num_crtc;

	int nconnectors = old_state->dev->mode_config.num_connector;

	int i;



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

	for (i = 0; i < old_state->num_connector; i++) {

		struct drm_connector_state *old_conn_state;

		struct drm_connector *connector;

		struct drm_encoder_helper_funcs *funcs;
@@ -553,12 +550,11 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) 
static void

set_routing_links(struct drm_device *dev, struct drm_atomic_state *old_state)

{

	int nconnectors = dev->mode_config.num_connector;

	int ncrtcs = old_state->dev->mode_config.num_crtc;

	int i;



	/* clear out existing links */

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

	for (i = 0; i < old_state->num_connector; i++) {

		struct drm_connector *connector;



		connector = old_state->connectors[i];
@@ -573,7 +569,7 @@ set_routing_links(struct drm_device *dev, struct drm_atomic_state *old_state) 
	}



	/* set new links */

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

	for (i = 0; i < old_state->num_connector; i++) {

		struct drm_connector *connector;



		connector = old_state->connectors[i];
@@ -608,7 +604,6 @@ static void 
crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)

{

	int ncrtcs = old_state->dev->mode_config.num_crtc;

	int nconnectors = old_state->dev->mode_config.num_connector;

	int i;



	for (i = 0; i < ncrtcs; i++) {
@@ -626,7 +621,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state) 
			funcs->mode_set_nofb(crtc);

	}



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

	for (i = 0; i < old_state->num_connector; i++) {

		struct drm_connector *connector;

		struct drm_crtc_state *new_crtc_state;

		struct drm_encoder_helper_funcs *funcs;
@@ -687,7 +682,6 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev, 
					  struct drm_atomic_state *old_state)

{

	int ncrtcs = old_state->dev->mode_config.num_crtc;

	int nconnectors = old_state->dev->mode_config.num_connector;

	int i;



	for (i = 0; i < ncrtcs; i++) {
@@ -706,7 +700,7 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev, 
			funcs->commit(crtc);

	}



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

	for (i = 0; i < old_state->num_connector; i++) {

		struct drm_connector *connector;

		struct drm_encoder_helper_funcs *funcs;

		struct drm_encoder *encoder;
@@ -1304,7 +1298,6 @@ static int update_output_state(struct drm_atomic_state *state, 
{

	struct drm_device *dev = set->crtc->dev;

	struct drm_connector_state *conn_state;

	int nconnectors = state->dev->mode_config.num_connector;

	int ncrtcs = state->dev->mode_config.num_crtc;

	int ret, i, j;
@@ -1333,7 +1326,7 @@ static int update_output_state(struct drm_atomic_state *state, 
	}



	/* Then recompute connector->crtc links and crtc enabling state. */

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

	for (i = 0; i < state->num_connector; i++) {

		struct drm_connector *connector;



		connector = state->connectors[i];

include/drm/drm_crtc.h

+2 −0
Original line number Diff line number Diff line
@@ -825,6 +825,7 @@ struct drm_bridge { 
 * @plane_states: pointer to array of plane states pointers

 * @crtcs: pointer to array of CRTC pointers

 * @crtc_states: pointer to array of CRTC states pointers

 * @num_connector: size of the @connectors and @connector_states arrays

 * @connectors: pointer to array of connector pointers

 * @connector_states: pointer to array of connector states pointers

 * @acquire_ctx: acquire context for this atomic modeset state update
@@ -836,6 +837,7 @@ struct drm_atomic_state { 
	struct drm_plane_state **plane_states;

	struct drm_crtc **crtcs;

	struct drm_crtc_state **crtc_states;

	int num_connector;

	struct drm_connector **connectors;

	struct drm_connector_state **connector_states;

CRA Git | Maintained and supported by SUSTech CRA and CCSE