Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm/i915/hdcp: stale type1_unsupported cache on MST hotplug - [Medium] drm/i915/hdcp: bypass of Type 1 fast-fail validation --
commit fb3d7b407b5fff7eb8ca9312ac9476d293a32b63 Author: George D. Sworo <[email protected]> drm/i915/hdcp: Fail fast when HDCP Type 1 is unsupported via MST This commit attempts to fail early in the atomic check phase when userspace requests HDCP Content Type 1 for a downstream MST sink that lacks HDCP 2.x support. It queries and caches the sink's capability during initialization and uses the cached result to reject unsupported configurations. > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > b/drivers/gpu/drm/i915/display/intel_hdcp.c > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c [ ... ] > @@ -2421,6 +2454,9 @@ int intel_hdcp_init(struct intel_connector *connector, > if (is_hdcp2_supported(display)) > intel_hdcp2_init(connector, dig_port, shim); > > + /* Cache the capability for the currently attached MST sink only. */ > + intel_hdcp_mst_update_type1_capability(connector, shim); > + [Severity: High] Could this cached capability become permanently stale after a hotplug event? Since DRM MST connectors represent topological branch ports rather than the sinks themselves, they persist across sink hotplug events. If a user connects an HDCP 1.x monitor, type1_unsupported will be cached as true here in intel_hdcp_init(). If the user then unplugs that monitor and connects an HDCP 2.x capable monitor to the same port, the cached value is never updated. Will this cause valid HDCP Type 1 requests to be permanently and incorrectly rejected for the new monitor? [ ... ] > @@ -2700,7 +2739,20 @@ void intel_hdcp_atomic_check(struct drm_connector > *connector, > if (old_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED) > new_state->content_protection = > DRM_MODE_CONTENT_PROTECTION_DESIRED; > - return; > + return 0; > + } > + > + /* > + * Fail fast if userspace asks for Type1 but neither the platform nor > + * the downstream sink can do HDCP 2.x, instead of only discovering > + * this once intel_hdcp_enable() is reached. > + */ > + if (new_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED && > + new_state->hdcp_content_type == DRM_MODE_HDCP_CONTENT_TYPE1 && > + intel_connector->mst.type1_unsupported) { [Severity: Medium] Does this condition inadvertently allow invalid configurations to bypass the fast-fail validation? If userspace has already enabled HDCP Type 0, the content_protection property will be in the ENABLED state. If a subsequent atomic commit changes only the hdcp_content_type to TYPE1, new_cp will inherit the ENABLED state. Since this check strictly requires new_cp to be DESIRED, it appears it will evaluate to false, allowing the invalid request to proceed and fail asynchronously during hardware enablement later. Should this check also account for the ENABLED state? > + drm_dbg_kms(connector->dev, > + "HDCP Type1 requested without HDCP 2.x support\n"); > + return -EOPNOTSUPP; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
