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: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>

3 files changed, 31 insertions, 20 deletions

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 67c1dc894bd9..3624632084e2 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -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];
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 0cd054615920..99095ef147ef 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -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)
 {