Thank you for your contribution! Sashiko AI review found 3 potential issue(s) 
to consider:
- [High] Use-After-Free during HDMI hotplug events due to vulnerable sysfs 
unbind path.
- [High] Function signature mismatch in `hdmi5_bridge_hpd_notify` causes a CFI 
kernel panic and breaks state reporting.
- [Medium] Missing initial jack state synchronization causes HDMI audio to 
remain disconnected at boot.
--

--- Patch [1]: [PATCH v2] drm/omap: Report HDMI hotplug events to the ASoC HDMI 
codec ---
commit 79155722ff98d49bdbf6c4e880ed9ec3b85d6fd0
Author: Ivaylo Dimitrov <[email protected]>

drm/omap: Report HDMI hotplug events to the ASoC HDMI codec

The OMAP HDMI audio driver currently has no way of reporting HDMI cable
hotplug events to ASoC. As a result, user space cannot detect HDMI audio
availability through the standard jack mechanism and audio policy managers
cannot automatically route audio to or from the HDMI output.

> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c 
> b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> index ab2d4eb6787f2..76aedc1c9ba1f 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> @@ -425,6 +425,14 @@ static void hdmi5_bridge_disable(struct drm_bridge 
> *bridge,
>       mutex_unlock(&hdmi->lock);
>  }
>  
> +static void hdmi5_bridge_hpd_notify(struct drm_bridge *bridge,
> +                                 enum drm_connector_status status)
> +{

[Severity: High]
Will this cause a Control-Flow Integrity (CFI) kernel panic?

The DRM core expects the hpd_notify callback to have three arguments:
void (*hpd_notify)(struct drm_bridge *bridge, struct drm_connector *connector,
                   enum drm_connector_status status)

hdmi5_bridge_hpd_notify() is defined with only two arguments. With CFI
enabled, calling this with a mismatched signature triggers a panic. Even
without CFI, the calling convention causes the callee to read the
connector pointer as the status enum.

> +     struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
> +
> +     hdmi_audio_hpd_notify(hdmi, status);
> +}
> +

[ ... ]

> diff --git a/sound/soc/ti/omap-hdmi.c b/sound/soc/ti/omap-hdmi.c
> index e60f5b483fc57..90ce12a812468 100644
> --- a/sound/soc/ti/omap-hdmi.c
> +++ b/sound/soc/ti/omap-hdmi.c
> @@ -262,6 +265,34 @@ static void hdmi_dai_shutdown(struct snd_pcm_substream 
> *substream,

[ ... ]

> +static int hdmi_audio_dai_init(struct snd_soc_pcm_runtime *rtd)
> +{
> +     struct snd_soc_card *card = rtd->card;
> +     struct hdmi_audio_data *ad = snd_soc_card_get_drvdata(rtd->card);
> +     int ret;
> +
> +     ret = snd_soc_card_jack_new(
> +                   card, "HDMI", SND_JACK_AVOUT, &ad->jack);

[Severity: Medium]
Is there a way to query the initial connection state of the jack here?

If the system boots with the HDMI cable already connected, the ASoC jack
is initialized to a disconnected state (0) here. Because the DRM hardware
interrupt only fires on state edges, hdmi_audio_hpd_notify() is never
called at boot, leaving the jack reporting as disconnected until the user
unplugs and replugs the cable.

> +     if (ret < 0) {
> +             dev_err(card->dev, "Cannot create HDMI jack: %i\n", ret);
> +             return ret;
> +     }
> +
> +     return 0;
> +}

[ ... ]

> @@ -371,28 +402,44 @@ static int omap_hdmi_audio_probe(struct platform_device 
> *pdev)

[ ... ]

> +static void omap_hdmi_audio_remove(struct platform_device *pdev)
> +{
> +     struct omap_hdmi_audio_pdata *ha = pdev->dev.platform_data;
> +
> +     /*
> +      * hdmi4_unbind() holds audio lock across platform_device_unregister(),
> +      * so audio_hpd() cannot run concurrently with this callback.
> +      */
> +     ha->audio_hpd = NULL;
> +}
> +
>  static struct platform_driver hdmi_audio_driver = {
>       .driver = {
>               .name = DRV_NAME,
>       },
>       .probe = omap_hdmi_audio_probe,
> +     .remove = omap_hdmi_audio_remove,

[Severity: High]
Could a user space sysfs unbind trigger a use-after-free here?

If a user unbinds the omap-hdmi-audio driver via sysfs,
omap_hdmi_audio_remove() locklessly clears ha->audio_hpd.

Concurrently, if a physical hotplug event occurs:

CPU1
hdmi_audio_hpd_notify()
    if (ha->audio_hpd)
        ...gets preempted here...

CPU2
omap_hdmi_audio_remove()
    ha->audio_hpd = NULL;
    ...driver unbinds and frees 'ad' via devres...

CPU1
    ...resumes...
    ha->audio_hpd(dev, ...) -> calls hdmi_audio_hpd()
    dereferences freed 'ad'.

Should this driver struct set .suppress_bind_attrs = true to prevent
uncoordinated sysfs lifecycle events?

>  };
>  
>  module_platform_driver(hdmi_audio_driver);

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

Reply via email to