On 13/06/2024 09:27, Péter Ujfalusi wrote: > > > 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@xxxxxxxxxx> >>> Link: https://github.com/thesofproject/avs-topology-xml/issues/22#issuecomment-2127892605 >>> Reviewed-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx> >>> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx> >>> --- >> >> 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@xxxxxxxxxxxxxxx> >> 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) or better yet: route->sink = devm_kasprintf(tplg->dev, GFP_KERNEL, "%s", elem->sink); > >>> + 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; >>> - else >>> - route->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@xxxxxxxxxxxxxxx> >> 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@xxxxxxxxxx> >> Link: >> https://github.com/thesofproject/avs-topology-xml/issues/22#issuecomment-2127892605 >> Reviewed-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx> >> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx> >> Link: >> https://lore.kernel.org/r/20240603102818.36165-2-amadeuszx.slawinski@xxxxxxxxxxxxxxx >> Signed-off-by: Mark Brown <broonie@xxxxxxxxxx> >> >> sound/soc/soc-topology.c | 27 ++++++++++++++++++++++----- >> 1 file changed, 22 insertions(+), 5 deletions(-) >> >> > -- Péter
