[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@xxxxxxxxx>
---
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)