[PATCH] drm/vc4: hdmi: Unregister the ASoC card on unbind

[PATCH] drm/vc4: hdmi: Unregister the ASoC card on unbind

[Karl Mehltretter]

vc4_hdmi, including the embedded ASoC card, is DRM-managed and freed
together with the DRM device when the aggregate device is torn down.
The card however is registered device-managed on the HDMI platform
device, whose release runs later, so snd_soc_unregister_card() operates
on freed memory:

  # modprobe vc4; rmmod vc4
  BUG: KASAN: slab-use-after-free in snd_soc_unregister_card
  Read of size 1 at addr ffff0000456a8450 by task rmmod/262
  Allocated by task 171: drmm_kmalloc / vc4_hdmi_bind
  Freed by task 262 (rmmod): drm_managed_release / drm_dev_put

Register the card without devm and unregister it from a component
unbind callback, where the HDMI device resources and the DRM-managed
structure are both still alive.

Fixes: b4f2c70c1a7a ("drm/vc4: hdmi: Switch to drmm_kzalloc")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
Tested on a Raspberry Pi 400 (BCM2711), v7.2-11658-g26260251022f, with
KASAN: report gone, rmmod/insmod loop clean.

 drivers/gpu/drm/vc4/vc4_hdmi.c | 46 ++++++++++++++++------------------
 drivers/gpu/drm/vc4/vc4_hdmi.h |  1 +
 2 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 17c8635c5afa..ce28075f77b6 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -2349,28 +2349,11 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
 	vc4_hdmi->audio.dma_data.maxburst = 2;
 
 	/*
-	 * NOTE: Strictly speaking, we should probably use a DRM-managed
-	 * registration there to avoid removing all the audio components
-	 * by the time the driver doesn't have any user anymore.
-	 *
-	 * However, the ASoC core uses a number of devm_kzalloc calls
-	 * when registering, even when using non-device-managed
-	 * functions (such as in snd_soc_register_component()).
-	 *
-	 * If we call snd_soc_unregister_component() in a DRM-managed
-	 * action, the device-managed actions have already been executed
-	 * and thus we would access memory that has been freed.
-	 *
-	 * Using device-managed hooks here probably leaves us open to a
-	 * bunch of issues if userspace still has a handle on the ALSA
-	 * device when the device is removed. However, this is mitigated
-	 * by the use of drm_dev_enter()/drm_dev_exit() in the audio
-	 * path to prevent the access to the device resources if it
-	 * isn't there anymore.
-	 *
-	 * Then, the vc4_hdmi structure is DRM-managed and thus only
-	 * freed whenever the last user has closed the DRM device file.
-	 * It should thus outlive ALSA in most situations.
+	 * The card is unregistered from the component unbind callback:
+	 * a DRM-managed action can run after the device-managed ASoC
+	 * resources are gone, and the device-managed release runs after
+	 * the DRM-managed vc4_hdmi structure holding the card has been
+	 * freed. Only at unbind time are both still alive.
 	 */
 	ret = devm_snd_dmaengine_pcm_register(dev, &pcm_conf, 0);
 	if (ret) {
@@ -2422,12 +2405,13 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)
 	 * snd_soc_card_get_drvdata() if needed.
 	 */
 	snd_soc_card_set_drvdata(card, vc4_hdmi);
-	ret = devm_snd_soc_register_card(dev, card);
+	ret = snd_soc_register_card(card);
 	if (ret)
-		dev_err_probe(dev, ret, "Could not register sound card\n");
+		return dev_err_probe(dev, ret, "Could not register sound card\n");
 
-	return ret;
+	vc4_hdmi->audio.card_registered = true;
 
+	return 0;
 }
 
 static irqreturn_t vc4_hdmi_hpd_irq_thread(int irq, void *priv)
@@ -3345,8 +3329,20 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 	return ret;
 }
 
+static void vc4_hdmi_unbind(struct device *dev, struct device *master,
+			    void *data)
+{
+	struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
+
+	if (vc4_hdmi->audio.card_registered) {
+		snd_soc_unregister_card(&vc4_hdmi->audio.card);
+		vc4_hdmi->audio.card_registered = false;
+	}
+}
+
 static const struct component_ops vc4_hdmi_ops = {
 	.bind   = vc4_hdmi_bind,
+	.unbind = vc4_hdmi_unbind,
 };
 
 static int vc4_hdmi_dev_probe(struct platform_device *pdev)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 29d461d4ee49..444c73513d86 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -106,6 +106,7 @@ struct vc4_hdmi_audio {
 	struct snd_soc_dai_link_component platform;
 	struct snd_dmaengine_dai_dma_data dma_data;
 	bool streaming;
+	bool card_registered;
 };
 
 /* General HDMI hardware state. */
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] drm/vc4: hdmi: Unregister the ASoC card on unbind

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete Fix / Use-After-Free / Resource Leak: The patch fixes a Use-After-Free for the ASoC card by replacing `devm_snd_soc_register_card` with a non-devres registration managed in `.unbind`. However, it leaves numerous other `devm_` allocations in `vc4_hdmi_bind()`, `vc4_hdmi_audio_init()`, `vc4_hdmi_hotplug_init()`, and `vc4_hdmi_cec_register()` tied to the component's platform device (`dev`) while passing pointers to the `drmm_kzalloc`-allocated `vc4_hdmi` structure. When the DRM aggregate device is unbound (e.g., via `rmmod vc4`), the master device's release runs and frees `vc4_hdmi`. The platform device, whose lifecycle is separate, retains the `devm_` actions (like `vc4_hdmi_put_ddc` and IRQ handlers) in its devres queue. When the platform device is eventually removed, or if an active interrupt fires after the DRM device unbinds, these actions access the freed `vc4_hdmi` pointer, causing a Use-After-Free. Additionally, unbinding and rebinding the component will stack duplicate devres actions on the platform device, causing memory leaks.
--

--- Patch [1]: [PATCH] drm/vc4: hdmi: Unregister the ASoC card on unbind ---
commit 197ce798da149a07ddab65100d64653586ed2892
Author: Karl Mehltretter <kmehltretter@gmail.com>

drm/vc4: hdmi: Unregister the ASoC card on unbind

The patch fixes a use-after-free issue by switching the ASoC card
registration from devm to manual registration and unregistering it during
the component unbind callback.

> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 17c8635c5afae..ce28075f77b68 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -2349,28 +2349,11 @@ static int vc4_hdmi_audio_init(struct vc4_hdmi *vc4_hdmi)

[ ... ]

> +	 * The card is unregistered from the component unbind callback:
> +	 * a DRM-managed action can run after the device-managed ASoC
> +	 * resources are gone, and the device-managed release runs after
> +	 * the DRM-managed vc4_hdmi structure holding the card has been
> +	 * freed. Only at unbind time are both still alive.
>  	 */
>  	ret = devm_snd_dmaengine_pcm_register(dev, &pcm_conf, 0);

[Severity: High]
This isn't a bug introduced by this patch, but while this commit fixes the UAF
for the ASoC card, does leaving devm_snd_dmaengine_pcm_register() and
devm_snd_soc_register_component() tied to the platform device (dev) leave
them vulnerable to a similar issue?

When the DRM master device is unbound, the drmm-managed vc4_hdmi structure
is freed. However, the HDMI platform device has a separate lifecycle, so these
devm_ resources won't be released until the platform device is removed. 

If the component is unbound and rebound, wouldn't these devm_ registrations
stack and leak memory?

[ ... ]

> @@ -3345,8 +3329,20 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
>  	return ret;
>  }
>  
> +static void vc4_hdmi_unbind(struct device *dev, struct device *master,
> +			    void *data)
> +{
> +	struct vc4_hdmi *vc4_hdmi = dev_get_drvdata(dev);
> +
> +	if (vc4_hdmi->audio.card_registered) {
> +		snd_soc_unregister_card(&vc4_hdmi->audio.card);
> +		vc4_hdmi->audio.card_registered = false;
> +	}
> +}

[Severity: High]
This is a pre-existing issue, but since the vc4_hdmi structure is passed
to several devm_ actions in the bind path (such as vc4_hdmi_put_ddc, and
IRQ handlers for HPD and CEC), wouldn't those callbacks dereference the
freed vc4_hdmi pointer if the platform device is removed after the DRM
master device has already been torn down?

For example, vc4_hdmi_hotplug_init() registers the HPD irq handler using
devm_request_threaded_irq() against the platform device. If an interrupt
fires after the DRM device is unbound, won't it try to access the freed
vc4_hdmi pointer?

Should these other resources also be manually cleaned up in the .unbind
callback, or transitioned to drmm_ management?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260822143218.68764-1-kmehltretter@gmail.com?part=1

Re: [PATCH] drm/vc4: hdmi: Unregister the ASoC card on unbind

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]

On Sat, Aug 22, 2026 at 04:32:18PM +0200, Karl Mehltretter wrote:
> vc4_hdmi, including the embedded ASoC card, is DRM-managed and freed
> together with the DRM device when the aggregate device is torn down.
> The card however is registered device-managed on the HDMI platform
> device, whose release runs later

Not necessarily, no. The comment you removed explained it. DRM-managed
memory isn't removed when the device in unbound/removed, but when it has
been and the last fd to the device file has been closed to userspace.

> so snd_soc_unregister_card() operates on freed memory:
> 
>   # modprobe vc4; rmmod vc4
>   BUG: KASAN: slab-use-after-free in snd_soc_unregister_card
>   Read of size 1 at addr ffff0000456a8450 by task rmmod/262
>   Allocated by task 171: drmm_kmalloc / vc4_hdmi_bind
>   Freed by task 262 (rmmod): drm_managed_release / drm_dev_put

And that happens only because you have no user. If you had one, you
would have had the opposite sequence.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH] drm/vc4: hdmi: Unregister the ASoC card on unbind

[Karl Mehltretter]

On Mon, Aug 24, 2026 at 08:52:44AM +0100, Maxime Ripard wrote:
> And that happens only because you have no user. If you had one, you
> would have had the opposite sequence.
> 

Hi Maxime,

Thanks for the correction, and sorry that v1 described the lifetime too
broadly.

You are right that DRM-managed memory is released when the last DRM
reference is gone, not simply at unbind. With an open DRM fd, vc4_hdmi can
outlive platform teardown.

v2 was triggered by syzbot comments on the ASoC retry path. A deferred card
bind can move the devm registration outside the component devres
group. With no DRM fd open, the final DRM release can then free vc4_hdmi
before the HDMI platform releases the card.

v2 link:
https://lore.kernel.org/r/20260823173740.2983-1-kmehltretter@gmail.com/

I will send a v3 based on your input.

The implementation change was already in v2. v3 only clarifies the wording
and keeps that same implementation. The card is registered without devm and
unregistered from component unbind while vc4_hdmi is still alive.

I also tested this on a real Pi 400 with /dev/dri/card0 open during vc4-drm
sysfs unbind. There were no new KASAN reports.

Thanks,
Karl