Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Missing validation of sink SCDC scrambling capabilities during the 
atomic check phase when scrambling is required.
--

commit caf66f1de17a42fc5664100ac7127160f89c2866
Author: Cristian Ciocaltea <[email protected]>

drm/display: hdmi-state-helper: Set HDMI scrambling requirement

This commit sets the HDMI scrambling requirement in the HDMI state helper
when the computed TMDS character rate exceeds the HDMI 1.3 maximum limit.
Centralizing this clock-based requirement prevents drivers from needing
to open-code the threshold check.

> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c 
> b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index 1e269b203b987..7cdb7ca3dc12f 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -596,6 +596,9 @@ hdmi_compute_clock(const struct drm_connector *connector,
>  
>       conn_state->hdmi.tmds_char_rate = clock;
>  
> +     /* TODO: also check drm_display_info.hdmi.scdc.scrambling.low_rates */
> +     conn_state->hdmi.scrambler_needed = (clock > 
> HDMI_1_3_TMDS_CHAR_RATE_MAX_HZ);

[Severity: High]
Does this allow an unsupported configuration to pass the atomic check phase?

When a high TMDS clock is requested, this unconditionally sets
conn_state->hdmi.scrambler_needed to true without validating sink SCDC
capabilities. The configuration can pass hdmi_compute_clock() and
atomic_check because info->max_tmds_clock is permitted to exceed 340MHz
regardless of SCDC support.

Later during the hardware programming phase, it will fail:

drivers/gpu/drm/display/drm_hdmi_helper.c:drm_connector_hdmi_enable_scrambling()
 {
        ...
        if (!drm_scdc_sink_supports_scrambling(connector)) {
                drm_dbg_kms(dev, "Sink doesn't support scrambling.\n");
                return -EINVAL;
        }
        ...
}

Failing during atomic_enable violates KMS atomic guarantees. This sequence
can be triggered remotely if an untrusted or malformed EDID advertises a high
TMDS clock via HF-VSDB but clears the SCDC supported bit.

Should this validate drm_scdc_sink_supports_scrambling(connector) during the
atomic check phase and return an error if scrambling is required but SCDC is
unsupported by the sink?

> +
>       return 0;
>  }
>

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=24

Reply via email to