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 <[email protected]>
---
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)
