Re: [PATCH v2 39/56] drm/i915/hdmi: Add support for sending uevent to user for FRL training failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/i915: missing drm_connector_put() in FRL modeset retry work causes refcount leak or UAF
- [High] drm/i915: refcount underflow and UAF in HDMI FRL modeset retry work
--

commit a06323f8771fc01d3d18f7c00ae7725f251aafec
Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

drm/i915/hdmi: Add support for sending uevent to user for FRL training failure

This commit adds an HDMI-specific work function for handling modeset retries
on link training failure. It also overrides the default connector work item
to use this new function when FRL is supported.

> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 0391597c9847f..69612ebe80f04 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -3397,6 +3399,31 @@ void intel_infoframe_init(struct intel_digital_port *dig_port)
>  	}
>  }
>  
> +/* Common code with DP, need to put in a common place */
> +static void intel_hdmi_modeset_retry_work_fn(struct work_struct *work)
> +{
> +	struct intel_connector *intel_connector;
> +	struct drm_connector *connector;
> +
> +	intel_connector = container_of(work, typeof(*intel_connector),
> +				       modeset_retry_work);
> +	connector = &intel_connector->base;
> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
> +		      connector->name);
> +
> +	/* Grab the locks before changing connector property*/
> +	mutex_lock(&connector->dev->mode_config.mutex);
> +	/*
> +	 * Set connector link status to BAD and send a Uevent to notify
> +	 * userspace to do a modeset.
> +	 */
> +	drm_connector_set_link_status_property(connector,
> +					       DRM_MODE_LINK_STATUS_BAD);
> +	mutex_unlock(&connector->dev->mode_config.mutex);
> +	/* Send Hotplug uevent so userspace can reprobe */
> +	drm_kms_helper_hotplug_event(connector->dev);
> +}

[Severity: High]
Does this function leak a connector reference, or leave the connector open to
a use-after-free?

The generic intel_connector_modeset_retry_work_fn expects a connector
reference to be taken before queueing, and balances it by calling
drm_connector_put() at the end of the work function.

This HDMI-specific function appears functionally identical to the generic
counterpart but omits the drm_connector_put() call.

If the work is queued via intel_connector_queue_modeset_retry_work(), which
acquires a reference, the missing put causes a permanent memory leak.

If the work is instead queued directly via schedule_work() without taking
a reference, no reference is held while the work executes. This allows a
concurrent teardown to free the connector out from under the running work
item.

[ ... ]

> @@ -3470,6 +3497,11 @@ bool intel_hdmi_init_connector(struct intel_digital_port *dig_port,
>  	if (!intel_hdmi->cec_notifier)
>  		drm_dbg_kms(display->drm, "CEC notifier get failed\n");
>  
> +	/* Initialize the work for modeset in case of link train failure */
> +	if (HAS_HDMI_FRL(display))
> +		INIT_WORK(&intel_connector->modeset_retry_work,
> +			  intel_hdmi_modeset_retry_work_fn);
> +

[Severity: High]
Does overriding the modeset_retry_work function here break the lifecycle
contract for the workqueue?

intel_connector_alloc() initializes modeset_retry_work with the generic
intel_connector_modeset_retry_work_fn, which manages connector refcounts.

If this work is queued directly using schedule_work() without acquiring a
connector reference first, and then cancelled during teardown via
intel_connector_cancel_modeset_retry_work() (called from
intel_hpd_poll_fini()), it will unconditionally call drm_connector_put()
upon successful cancellation.

Since no reference was acquired before scheduling the work, this cancellation
drops a reference that was never acquired, causing a refcount underflow and
premature freeing of the connector.

>  	return true;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012035806.3361439-1-ankit.k.nautiyal@intel.com?part=39