Skip to content

ASoC: SOF: Intel: hda: Fix NULL pointer dereference #5650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ranj063 wants to merge 3 commits into
base: topic/sof-dev
Choose a base branch
from
+23 −5
Open

ASoC: SOF: Intel: hda: Fix NULL pointer dereference #5650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
81ac3fa
ASoC: SOF: Intel: hda: Fix NULL pointer dereference
ranj063 Jan 15, 2026
d2671f1
ASoC: SOF: Intel: hda: Add a virtual CPU DAI
ranj063 Jan 16, 2026
b1a743e
fixup! ASoC: Intel: sof_sdw: Add a DAI link for loopback capture
ranj063 Jan 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sound/soc/intel/boards/sof_sdw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,7 @@ static int create_echoref_dailink(struct snd_soc_card *card,
* fe <-> be connection for loopback capture for echo reference
*/
ret = asoc_sdw_init_simple_dai_link(dev, *dai_links, be_id, name,
0, 1, "snd-soc-dummy-dai", "dummy",
0, 1, "Loopback Virtual Pin", "dummy",
Copy link
Collaborator

@ujfalusi ujfalusi Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Collaborator Author

@ranj063 ranj063 Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ujfalusi this is to fix a conflict with BT. The BT DAI link already uses the dummy DAI and is created before the echo ref DAI in the machine driver. So ifyou use the same dummy CPU DAI for echo ref, the BT DAI link gets picked when connecting FE and BE with DPCM. One way to fix this is to create the echo ref first but then BT will be broken. So using the dummy DAI for BT and the virtual pin for echo ref is the right way to do it so that the 2 features are not mutually exclusive

Copy link
Collaborator

@ujfalusi ujfalusi Jan 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you saying that if we resort to use dummy-dai for HDMI (because no iDisp codec) then BT will fail today also?
And this is the CPU dai, which for BT is always "SSP%d Pin", in fact I don't see in any asoc_sdw_init_simple_dai_link() that the "snd-soc-dummy-dai" is used, but in the echoref case.

Something is not adding up.

snd_soc_dummy_dlc.name, snd_soc_dummy_dlc.dai_name,
1, NULL, NULL);
if (ret)
Expand Down
22 changes: 20 additions & 2 deletions sound/soc/sof/intel/hda-dai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,22 @@ static const struct hda_dai_widget_dma_ops *
hda_dai_get_ops(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai)
{
struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream);
struct snd_sof_widget *swidget = w->dobj.private;
struct snd_sof_widget *swidget;
struct snd_sof_dev *sdev;
struct snd_sof_dai *sdai;

sdev = widget_to_sdev(w);
/*
* this is unlikely if the topology and the machine driver DAI links match.
* But if there's a missing DAI link in topology, this will prevent a NULL pointer
* dereference later on.
*/
Copy link
Collaborator

@lyakh lyakh Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is such a case viable? Shouldn't it be caught before this call, maybe even in the driver? But maybe this is indeed the first location where such a check is feasible, not sure really, just clarifying

Copy link
Collaborator Author

@ranj063 ranj063 Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyakh if the topology is clean and it matches the machine driver exactly, this isnt possible but I encountered the problem with echo ref defined in the machine driver and topology but BT only in the machine driver. This will not happen in production topologies but even for development topologies an error is more graceful than a crash

Copy link
Collaborator

@ujfalusi ujfalusi Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, but how it works w/o the dai less stuff?
I have devices with BT link, but none of the topology I have uses them and yet, no NULL pointer.

We also have this exact case supported with function topologies when there is no monolithic present: we skip loading fragment for non supported features (like BT) to form the card, because none of the generic topologgies that we have supports BT link.

Is this a fixup for the dai less series?

if (!w) {
dev_err(cpu_dai->dev, "%s: widget is NULL\n", __func__);
return NULL;
}

sdev = widget_to_sdev(w);
swidget = w->dobj.private;
if (!swidget) {
dev_err(sdev->dev, "%s: swidget is NULL\n", __func__);
return NULL;
Expand Down Expand Up @@ -856,6 +866,14 @@ struct snd_soc_dai_driver skl_dai[] = {
.channels_max = 4,
},
},
{
/* Virtual CPU DAI for Echo reference */
.name = "Loopback Virtual Pin",
.capture = {
.channels_min = 1,
.channels_max = 2,
},
},
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC)
{
.name = "iDisp1 Pin",
Expand Down
4 changes: 2 additions & 2 deletions sound/soc/sof/intel/hda.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,10 +418,10 @@
(HDA_DSP_BDL_SIZE / sizeof(struct sof_intel_dsp_bdl))

/* Number of DAIs */
#define SOF_SKL_NUM_DAIS_NOCODEC 8
#define SOF_SKL_NUM_DAIS_NOCODEC 9

#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC)
#define SOF_SKL_NUM_DAIS 15
#define SOF_SKL_NUM_DAIS 16
Copy link
Collaborator

@ujfalusi ujfalusi Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a fixup or should be collected together with the dai less series?

Copy link
Collaborator Author

@ranj063 ranj063 Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a fixup, keep this as a separate patch in the DAIless series at the beginning

#else
#define SOF_SKL_NUM_DAIS SOF_SKL_NUM_DAIS_NOCODEC
#endif
Expand Down
Loading