| Message ID | 20240924153022.2255299-1-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/hdcp: fix connector refcounting | expand |
> -----Original Message----- > From: Nikula, Jani <jani.nikula@intel.com> > Sent: Tuesday, September 24, 2024 9:00 PM > To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org > Cc: Nikula, Jani <jani.nikula@intel.com>; Sean Paul > <seanpaul@chromium.org>; Kandpal, Suraj <suraj.kandpal@intel.com>; > Ville Syrjälä <ville.syrjala@linux.intel.com>; stable@vger.kernel.org > Subject: [PATCH] drm/i915/hdcp: fix connector refcounting > > We acquire a connector reference before scheduling an HDCP prop work, > and expect the work function to release the reference. > > However, if the work was already queued, it won't be queued multiple > times, and the reference is not dropped. > > Release the reference immediately if the work was already queued. > LGTM, Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> > Fixes: a6597faa2d59 ("drm/i915: Protect workers against disappearing > connectors") > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Suraj Kandpal <suraj.kandpal@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: <stable@vger.kernel.org> # v5.10+ > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > --- > > I don't know that we have any bugs open about this. Or how it would > manifest itself. Memory leak on driver unload? I just spotted this while > reading the code for other reasons. > --- > drivers/gpu/drm/i915/display/intel_hdcp.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > b/drivers/gpu/drm/i915/display/intel_hdcp.c > index 2afa92321b08..cad309602617 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -1097,7 +1097,8 @@ static void intel_hdcp_update_value(struct > intel_connector *connector, > hdcp->value = value; > if (update_property) { > drm_connector_get(&connector->base); > - queue_work(i915->unordered_wq, &hdcp->prop_work); > + if (!queue_work(i915->unordered_wq, &hdcp->prop_work)) > + drm_connector_put(&connector->base); > } > } > > @@ -2531,7 +2532,8 @@ void intel_hdcp_update_pipe(struct > intel_atomic_state *state, > mutex_lock(&hdcp->mutex); > hdcp->value = > DRM_MODE_CONTENT_PROTECTION_DESIRED; > drm_connector_get(&connector->base); > - queue_work(i915->unordered_wq, &hdcp->prop_work); > + if (!queue_work(i915->unordered_wq, &hdcp->prop_work)) > + drm_connector_put(&connector->base); > mutex_unlock(&hdcp->mutex); > } > > @@ -2548,7 +2550,9 @@ void intel_hdcp_update_pipe(struct > intel_atomic_state *state, > */ > if (!desired_and_not_enabled && > !content_protection_type_changed) { > drm_connector_get(&connector->base); > - queue_work(i915->unordered_wq, &hdcp- > >prop_work); > + if (!queue_work(i915->unordered_wq, &hdcp- > >prop_work)) > + drm_connector_put(&connector->base); > + > } > } > > -- > 2.39.2
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 2afa92321b08..cad309602617 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -1097,7 +1097,8 @@ static void intel_hdcp_update_value(struct intel_connector *connector, hdcp->value = value; if (update_property) { drm_connector_get(&connector->base); - queue_work(i915->unordered_wq, &hdcp->prop_work); + if (!queue_work(i915->unordered_wq, &hdcp->prop_work)) + drm_connector_put(&connector->base); } } @@ -2531,7 +2532,8 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state, mutex_lock(&hdcp->mutex); hdcp->value = DRM_MODE_CONTENT_PROTECTION_DESIRED; drm_connector_get(&connector->base); - queue_work(i915->unordered_wq, &hdcp->prop_work); + if (!queue_work(i915->unordered_wq, &hdcp->prop_work)) + drm_connector_put(&connector->base); mutex_unlock(&hdcp->mutex); } @@ -2548,7 +2550,9 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state, */ if (!desired_and_not_enabled && !content_protection_type_changed) { drm_connector_get(&connector->base); - queue_work(i915->unordered_wq, &hdcp->prop_work); + if (!queue_work(i915->unordered_wq, &hdcp->prop_work)) + drm_connector_put(&connector->base); + } }
We acquire a connector reference before scheduling an HDCP prop work, and expect the work function to release the reference. However, if the work was already queued, it won't be queued multiple times, and the reference is not dropped. Release the reference immediately if the work was already queued. Fixes: a6597faa2d59 ("drm/i915: Protect workers against disappearing connectors") Cc: Sean Paul <seanpaul@chromium.org> Cc: Suraj Kandpal <suraj.kandpal@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: <stable@vger.kernel.org> # v5.10+ Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- I don't know that we have any bugs open about this. Or how it would manifest itself. Memory leak on driver unload? I just spotted this while reading the code for other reasons. --- drivers/gpu/drm/i915/display/intel_hdcp.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)