On 13/06/2024 08:58, Pierre-Louis Bossart wrote:
On 6/3/24 12:28, Amadeusz Sławiński wrote:
Most users after parsing a topology file, release memory used by it, so having pointer references directly into topology file contents is wrong. Use devm_kmemdup(), to allocate memory as needed.
Reported-by: Jason Montleon jmontleo@redhat.com Link: https://github.com/thesofproject/avs-topology-xml/issues/22#issuecomment-212... Reviewed-by: Cezary Rojewski cezary.rojewski@intel.com Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com
This patch breaks the Intel SOF CI in spectacular ways, with the widgets names completely garbled with noise such as
host-copier.5.playbackpid.socket host-copier.5.playbackrt@linux.intel.com> dai-copier.HDA.iDisp3.playbackrun_t:s0 host-copier.31.playback\xff`\x86\xba\x034\x89\xff\xff@N\x83\xb83\x89\xff\xff\x10\x84\xe9\x8b\xff\xff\xff\xffS\x81ی\xff\xff\xff\xff\x0f
https://github.com/thesofproject/linux/pull/5057#issuecomment-2164470192
I am going to revert this patchset in the SOF tree.
sound/soc/soc-topology.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 90ca37e008b32..75d9395a18ed4 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -1060,15 +1060,32 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg, break; }
route->source = elem->source;route->sink = elem->sink;
route->source = devm_kmemdup(tplg->dev, elem->source,min(strlen(elem->source),SNDRV_CTL_ELEM_ID_NAME_MAXLEN),GFP_KERNEL);route->sink = devm_kmemdup(tplg->dev, elem->sink,min(strlen(elem->sink), SNDRV_CTL_ELEM_ID_NAME_MAXLEN),
Initially I did not see why this breaks, but then:
The strlen() function calculates the length of the string pointed to by s, excluding the terminating null byte ('\0').
Likely the fix is as simple as: min(strlen(elem->sink) + 1, SNDRV_CTL_ELEM_ID_NAME_MAXLEN)
GFP_KERNEL);if (!route->source || !route->sink) {ret = -ENOMEM;break;}/* set to NULL atm for tplg users */ route->connected = NULL;
if (strnlen(elem->control, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == 0)
if (strnlen(elem->control, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == 0) { route->control = NULL;
elseroute->control = elem->control;
} else {route->control = devm_kmemdup(tplg->dev, elem->control,min(strlen(elem->control),SNDRV_CTL_ELEM_ID_NAME_MAXLEN),GFP_KERNEL);if (!route->control) {ret = -ENOMEM;break;}}/* add route dobj to dobj_list */ route->dobj.type = SND_SOC_DOBJ_GRAPH;
97ab304ecd95c0b1703ff8c8c3956dc6e2afe8e1 is the first bad commit commit 97ab304ecd95c0b1703ff8c8c3956dc6e2afe8e1 Author: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com Date: Mon Jun 3 12:28:15 2024 +0200
ASoC: topology: Fix references to freed memory Most users after parsing a topology file, release memory used by it, so having pointer references directly into topology file contents is wrong. Use devm_kmemdup(), to allocate memory as needed. Reported-by: Jason Montleon <jmontleo@redhat.com> Link:https://github.com/thesofproject/avs-topology-xml/issues/22#issuecomment-212... Reviewed-by: Cezary Rojewski cezary.rojewski@intel.com Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com Link: https://lore.kernel.org/r/20240603102818.36165-2-amadeuszx.slawinski@linux.i... Signed-off-by: Mark Brown broonie@kernel.org
sound/soc/soc-topology.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-)