To provide backward compatibility to older systems, the SOF HDA driver allows user to specify which HDMI codec driver to use at runtime via kernel parameter. This mechanism has a subtle flaw in that it assumes the codec drivers not to be loaded when the SOF PCI driver is loaded. The problem is rooted in use of the hdev->type field. snd_hdac_ext_bus_device_init() initializes this field to HDA_DEV_ASOC. This signals the HDA core that ASoC drivers should be considered in driver matching (hda_bus_match()). The SOF and SST drivers continue by overriding this field to HDA_DEV_LEGACY and proceeding to load driver modules with request_module(). Correct drivers will get loaded and attached. If however the codec drivers are already loaded when snd_hdac_ext_bus_device_init() is called, the matching will not work as expected as device type is still set to HDA_DEV_ASOC. Specifically if hdac-hdmi is attached when machine driver is configured to use hdac-hda, this leads to out-of-bounds memory access in hda_dsp_hdmi_build_controls(). Fix the issue by adding codec type as a parameter to snd_hdac_ext_bus_device_init() and ensuring type is set correctly from the start. BugLink: https://github.com/thesofproject/linux/issues/2311 Fixes: 139c7febad1a ("ASoC: SOF: Intel: add support for snd-hda-codec-hdmi") Signed-off-by: Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx> Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@xxxxxxxxxxxxxxx> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> --- include/sound/hdaudio_ext.h | 2 +- sound/hda/ext/hdac_ext_bus.c | 5 +++-- sound/soc/intel/skylake/skl.c | 4 ++-- sound/soc/sof/intel/hda-codec.c | 17 ++++++++--------- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index ef88b20c7b0a..7abf74c1c474 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -10,7 +10,7 @@ int snd_hdac_ext_bus_init(struct hdac_bus *bus, struct device *dev, void snd_hdac_ext_bus_exit(struct hdac_bus *bus); int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr, - struct hdac_device *hdev); + struct hdac_device *hdev, int type); void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev); void snd_hdac_ext_bus_device_remove(struct hdac_bus *bus); diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c index d0a604c939df..765c40a6ccba 100644 --- a/sound/hda/ext/hdac_ext_bus.c +++ b/sound/hda/ext/hdac_ext_bus.c @@ -70,11 +70,12 @@ static void default_release(struct device *dev) * @bus: hdac bus to attach to * @addr: codec address * @hdev: hdac device to init + * @type: codec type (HDAC_DEV_*) to use for this device * * Returns zero for success or a negative error code. */ int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr, - struct hdac_device *hdev) + struct hdac_device *hdev, int type) { char name[15]; int ret; @@ -88,7 +89,7 @@ int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr, dev_err(bus->dev, "device init failed for hdac device\n"); return ret; } - hdev->type = HDA_DEV_ASOC; + hdev->type = type; hdev->dev.release = default_release; ret = snd_hdac_device_register(hdev); diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index ea00cf61d1a8..8b993722f74e 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -721,7 +721,7 @@ static int probe_codec(struct hdac_bus *bus, int addr) hda_codec->codec.bus = skl_to_hbus(skl); hdev = &hda_codec->codec.core; - err = snd_hdac_ext_bus_device_init(bus, addr, hdev); + err = snd_hdac_ext_bus_device_init(bus, addr, hdev, HDA_DEV_ASOC); if (err < 0) return err; @@ -736,7 +736,7 @@ static int probe_codec(struct hdac_bus *bus, int addr) if (!hdev) return -ENOMEM; - return snd_hdac_ext_bus_device_init(bus, addr, hdev); + return snd_hdac_ext_bus_device_init(bus, addr, hdev, HDA_DEV_ASOC); #endif /* CONFIG_SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC */ } diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c index f6ba3b593e1f..6875fa570c2c 100644 --- a/sound/soc/sof/intel/hda-codec.c +++ b/sound/soc/sof/intel/hda-codec.c @@ -117,6 +117,7 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address, #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC) struct hdac_hda_priv *hda_priv; struct hda_codec *codec; + int type = HDA_DEV_LEGACY; #endif struct hda_bus *hbus = sof_to_hbus(sdev); struct hdac_device *hdev; @@ -143,7 +144,11 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address, hdev = &hda_priv->codec.core; codec = &hda_priv->codec; - ret = snd_hdac_ext_bus_device_init(&hbus->core, address, hdev); + /* only probe ASoC codec drivers for HDAC-HDMI */ + if (!hda_codec_use_common_hdmi && (resp & 0xFFFF0000) == IDISP_VID_INTEL) + type = HDA_DEV_ASOC; + + ret = snd_hdac_ext_bus_device_init(&hbus->core, address, hdev, type); if (ret < 0) return ret; @@ -161,13 +166,7 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address, else codec->probe_id = 0; - /* - * if common HDMI codec driver is not used, codec load - * is skipped here and hdac_hdmi is used instead - */ - if (hda_codec_use_common_hdmi || - (resp & 0xFFFF0000) != IDISP_VID_INTEL) { - hdev->type = HDA_DEV_LEGACY; + if (type == HDA_DEV_LEGACY) { ret = hda_codec_load_module(codec); /* * handle ret==0 (no driver bound) as an error, but pass @@ -188,7 +187,7 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address, if (!hdev) return -ENOMEM; - ret = snd_hdac_ext_bus_device_init(&hbus->core, address, hdev); + ret = snd_hdac_ext_bus_device_init(&hbus->core, address, hdev, HDA_DEV_ASOC); return ret; #endif -- 2.27.0
