[PATCH] ASoC: hdac: make SOF HDA codec driver probe deterministic

[Kai Vehmanen]

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 at linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski at linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
---
 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