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
[Index of Archives]
[Pulseaudio]
[Linux Audio Users]
[ALSA Devel]
[Fedora Desktop]
[Fedora SELinux]
[Big List of Linux Books]
[Yosemite News]
[KDE Users]