Re: [PATCH AUTOSEL 6.9 17/40] ASoC: topology: Fix route memory corruption

Amadeusz Sławiński
2024-08-12T10:01:55Z

I guess that for completeness you need to apply both patches:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/sound/soc/soc-topology.c?id=97ab304ecd95c0b1703ff8c8c3956dc6e2afe8e1
was an incorrect fix which was later fixed by:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/sound/soc/soc-topology.c?id=0298f51652be47b79780833e0b63194e1231fa34

Applying just first one will result in runtime problems, while applying
just second one will result in missing NULL checks on allocation.

On 8/12/2024 11:53 AM, Thorsten Leemhuis wrote:
> Hi, top-posting for once, to make this easily accessible to everyone.
>
> Greg, Sasha, to me it looks like something fell through the cracks.
> Pierre-Louis afaics about a week ago asked (see the quote below) to
> revert 97ab304ecd95c0 ("ASoC: topology: Fix references to freed memory")
> [v6.10-rc6, v6.9.11, v6.6.42, v6.1.101] from the stable branches *or*
> pick up b9dd212b14d27a ("ASoC: topology: Fix route memory corruption").
> But nothing like that has happened yet and I can't see any of those
> resolutions in the 6.6 queue.
>
> Side note: I have a very strong feeling that I'm missing or
> misunderstood something, but I decided to send this mail despite this...
> If something like that was the case: apologies in advance.
>
> Ciao, Thorsten
>
> On 05.08.24 19:09, Pierre-Louis Bossart wrote:
>> On 8/5/24 18:17, Vitaly Chikunov wrote:
>>> Sasha, Greg,
>>>
>>> On Tue, Jul 09, 2024 at 12:18:57PM GMT, Sasha Levin wrote:
>>>> From: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>>>>
>>>> [ Upstream commit 0298f51652be47b79780833e0b63194e1231fa34 ]
>>>>
>>>> It was reported that recent fix for memory corruption during topology
>>>> load, causes corruption in other cases. Instead of being overeager with
>>>> checking topology, assume that it is properly formatted and just
>>>> duplicate strings.
>>>
>>> Can this backport actually be applied to the 6.9/6.6/6.1 stable branches?
>>>
>>> I have multiple bug reports about sound not working and memory
>>> corruption on some laptops (for example ICL RAYbook Si1516). See for
>>> example bug reports[1][2], and the fix discussion [3].
>>>
>>> dmesg messages from Lenovo ThinkBook 13 gen 1:
>>>
>>>
>>> [ 3.555191] sof-audio-pci-intel-cnl 0000:00:1f.3: Firmware info: version 2:2:0-57864
>>> [ 3.555206] sof-audio-pci-intel-cnl 0000:00:1f.3: Firmware: ABI 3:22:1 Kernel ABI 3:23:0
>>> [ 3.574043] sof-audio-pci-intel-cnl 0000:00:1f.3: Topology: ABI 3:22:1 Kernel ABI 3:23:0
>>> [ 3.575180] sof-audio-pci-intel-cnl 0000:00:1f.3: error: sink MIXER1.0> not found
>>> [ 3.575772] sof-audio-pci-intel-cnl 0000:00:1f.3: error: tplg component load failed -22
>>> [ 3.575793] sof-audio-pci-intel-cnl 0000:00:1f.3: error: failed to load DSP topology -22
>>> [ 3.575801] sof-audio-pci-intel-cnl 0000:00:1f.3: ASoC: error at snd_soc_component_probe on 0000:00:1f.3: -22
>>>
>>> Error messages from other boots showing memory corruption:
>>>
>>> [ 3.904397] sof-audio-pci-intel-cnl 0000:00:1f.3: error: sink PCM0C03-std-def-alt0.p11@jh\x86Ŝ\xff\xff@\xc8\xff\x82Ŝ\xff\xff`P\x82\xbb\xff\xff\xff\xff\x94$A\xbc\xff\xff\xff\xff\x06 not found
>>> [ 3.966777] sof-audio-pci-intel-cnl 0000:00:1f.3: error: sink PGA1.0\x01 not found
>>> [ 3.899748] sof-audio-pci-intel-cnl 0000:00:1f.3: error: source BUF2.0 not found
>>> [ 3.975359] sof-audio-pci-intel-cnl 0000:00:1f.3: error: source PCM0P\x01pcsc-lite.conf not found
>>> [ 7.275851] sof-audio-pci-intel-tgl 0000:00:1f.3: error: source HDA1.IN/0123456789:;<=>? not found
>>>
>>> [1] https://github.com/thesofproject/sof/issues/9339
>>> [2] https://github.com/thesofproject/sof/issues/9341
>>> [3] https://lore.kernel.org/linux-sound/171812236450.201359.3019210915105428447.b4-ty@kernel.org/T/#m8c4bd5abf453960fde6f826c4b7f84881da63e9d
>>
>> Agree, the commit "ASoC: topology: Fix references to freed memory"
>> [ Upstream commit 97ab304ecd95c0b1703ff8c8c3956dc6e2afe8e1 ]
>> should not have landed on any -stable branch. It should be reverted or
>> this follow-up fix be applied.
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>>> Closes: https://lore.kernel.org/linux-sound/171812236450.201359.3019210915105428447.b4-ty@kernel.org/T/#m8c4bd5abf453960fde6f826c4b7f84881da63e9d
>>>> Suggested-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
>>>> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>>>> Link: https://lore.kernel.org/r/20240613090126.841189-1-amadeuszx.slawinski@linux.intel.com
>>>> Signed-off-by: Mark Brown <broonie@kernel.org>
>>>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>>>> ---
>>>> sound/soc/soc-topology.c | 12 +++---------
>>>> 1 file changed, 3 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
>>>> index 52752e0a5dc27..27aba69894b17 100644
>>>> --- a/sound/soc/soc-topology.c
>>>> +++ b/sound/soc/soc-topology.c
>>>> @@ -1052,21 +1052,15 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
>>>> break;
>>>> }
>>>>
>>>> - route->source = devm_kmemdup(tplg->dev, elem->source,
>>>> - min(strlen(elem->source), maxlen),
>>>> - GFP_KERNEL);
>>>> - route->sink = devm_kmemdup(tplg->dev, elem->sink,
>>>> - min(strlen(elem->sink), maxlen),
>>>> - GFP_KERNEL);
>>>> + route->source = devm_kstrdup(tplg->dev, elem->source, GFP_KERNEL);
>>>> + route->sink = devm_kstrdup(tplg->dev, elem->sink, GFP_KERNEL);
>>>> if (!route->source || !route->sink) {
>>>> ret = -ENOMEM;
>>>> break;
>>>> }
>>>>
>>>> if (strnlen(elem->control, maxlen) != 0) {
>>>> - route->control = devm_kmemdup(tplg->dev, elem->control,
>>>> - min(strlen(elem->control), maxlen),
>>>> - GFP_KERNEL);
>>>> + route->control = devm_kstrdup(tplg->dev, elem->control, GFP_KERNEL);
>>>> if (!route->control) {
>>>> ret = -ENOMEM;
>>>> break;
>>>> --
>>>> 2.43.0
>>>>
>>