[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

[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