Commit a6597faa authored by Sean Paul's avatar Sean Paul Committed by Ramalingam C

drm/i915: Protect workers against disappearing connectors

This patch adds some protection against connectors being destroyed
before the HDCP workers are finished.

For check_work, we do a synchronous cancel after the connector is
unregistered which will ensure that it is finished before destruction.

In the case of prop_work, we can't do a synchronous wait since it needs
to take connection_mutex which could cause deadlock. Instead, we'll take
a reference on the connector when scheduling prop_work and give it up
once we're done.
Reviewed-by: default avatarRamalingam C <ramalingam.c@intel.com>
Signed-off-by: default avatarSean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20191212190230.188505-8-sean@poorly.run #v2
Link: https://patchwork.freedesktop.org/patch/msgid/20200117193103.156821-8-sean@poorly.run #v3
Link: https://patchwork.freedesktop.org/patch/msgid/20200218220242.107265-8-sean@poorly.run #v4
Link: https://patchwork.freedesktop.org/patch/msgid/20200305201236.152307-8-sean@poorly.run #v5
Link: https://patchwork.freedesktop.org/patch/msgid/20200429195502.39919-8-sean@poorly.run #v6
Link: https://patchwork.freedesktop.org/patch/msgid/20200623155907.22961-8-sean@poorly.run #v7

Changes in v2:
-Added to the set
Changes in v3:
-Change the WARN_ON condition in intel_hdcp_cleanup to allow for
 initializing connectors as well
Changes in v4:
-None
Changes in v5:
-Change WARN_ON to drm_WARN_ON
Changes in v6:
-None
Changes in v7:
-None
Changes in v8:
-None
Signed-off-by: default avatarRamalingam C <ramalingam.c@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200818153910.27894-8-sean@poorly.run
parent a72394e4
...@@ -878,8 +878,10 @@ static void intel_hdcp_update_value(struct intel_connector *connector, ...@@ -878,8 +878,10 @@ static void intel_hdcp_update_value(struct intel_connector *connector,
return; return;
hdcp->value = value; hdcp->value = value;
if (update_property) if (update_property) {
drm_connector_get(&connector->base);
schedule_work(&hdcp->prop_work); schedule_work(&hdcp->prop_work);
}
} }
/* Implements Part 3 of the HDCP authorization procedure */ /* Implements Part 3 of the HDCP authorization procedure */
...@@ -971,6 +973,8 @@ static void intel_hdcp_prop_work(struct work_struct *work) ...@@ -971,6 +973,8 @@ static void intel_hdcp_prop_work(struct work_struct *work)
mutex_unlock(&hdcp->mutex); mutex_unlock(&hdcp->mutex);
drm_modeset_unlock(&dev_priv->drm.mode_config.connection_mutex); drm_modeset_unlock(&dev_priv->drm.mode_config.connection_mutex);
drm_connector_put(&connector->base);
} }
bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port) bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port)
...@@ -1850,6 +1854,9 @@ static void intel_hdcp_check_work(struct work_struct *work) ...@@ -1850,6 +1854,9 @@ static void intel_hdcp_check_work(struct work_struct *work)
check_work); check_work);
struct intel_connector *connector = intel_hdcp_to_connector(hdcp); struct intel_connector *connector = intel_hdcp_to_connector(hdcp);
if (drm_connector_is_unregistered(&connector->base))
return;
if (!intel_hdcp2_check_link(connector)) if (!intel_hdcp2_check_link(connector))
schedule_delayed_work(&hdcp->check_work, schedule_delayed_work(&hdcp->check_work,
DRM_HDCP2_CHECK_PERIOD_MS); DRM_HDCP2_CHECK_PERIOD_MS);
...@@ -2180,12 +2187,39 @@ void intel_hdcp_component_fini(struct drm_i915_private *dev_priv) ...@@ -2180,12 +2187,39 @@ void intel_hdcp_component_fini(struct drm_i915_private *dev_priv)
void intel_hdcp_cleanup(struct intel_connector *connector) void intel_hdcp_cleanup(struct intel_connector *connector)
{ {
if (!connector->hdcp.shim) struct intel_hdcp *hdcp = &connector->hdcp;
if (!hdcp->shim)
return; return;
mutex_lock(&connector->hdcp.mutex); /*
kfree(connector->hdcp.port_data.streams); * If the connector is registered, it's possible userspace could kick
mutex_unlock(&connector->hdcp.mutex); * off another HDCP enable, which would re-spawn the workers.
*/
drm_WARN_ON(connector->base.dev,
connector->base.registration_state == DRM_CONNECTOR_REGISTERED);
/*
* Now that the connector is not registered, check_work won't be run,
* but cancel any outstanding instances of it
*/
cancel_delayed_work_sync(&hdcp->check_work);
/*
* We don't cancel prop_work in the same way as check_work since it
* requires connection_mutex which could be held while calling this
* function. Instead, we rely on the connector references grabbed before
* scheduling prop_work to ensure the connector is alive when prop_work
* is run. So if we're in the destroy path (which is where this
* function should be called), we're "guaranteed" that prop_work is not
* active (tl;dr This Should Never Happen).
*/
drm_WARN_ON(connector->base.dev, work_pending(&hdcp->prop_work));
mutex_lock(&hdcp->mutex);
kfree(hdcp->port_data.streams);
hdcp->shim = NULL;
mutex_unlock(&hdcp->mutex);
} }
void intel_hdcp_atomic_check(struct drm_connector *connector, void intel_hdcp_atomic_check(struct drm_connector *connector,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment