Commit 97ab304ecd95 ("ASoC: topology: Fix references to freed memory") is a problematic fix for issue in topology loading code, which was cherry-picked to stable. It was later corrected in 0298f51652be ("ASoC: topology: Fix route memory corruption"), however to apply cleanly e0e7bc2cbee9 ("ASoC: topology: Clean up route loading") also needs to be applied.
Link: https://lore.kernel.org/linux-sound/ZrwUCnrtKQ61LWFS@sashalap/T/#mbfd273adf8...
Should be applied to stable 6.1, 6.6, 6.9.
v2: - Mention base commit - Sign-off patches again, as those are cherrypicks
Amadeusz Sławiński (2): ASoC: topology: Clean up route loading ASoC: topology: Fix route memory corruption
sound/soc/soc-topology.c | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-)
[ Upstream commit e0e7bc2cbee93778c4ad7d9a792d425ffb5af6f7 ]
Instead of using very long macro name, assign it to shorter variable and use it instead. While doing that, we can reduce multiple if checks using this define to one.
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-5-amadeuszx.slawinski@linux.i... Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- sound/soc/soc-topology.c | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index ce22613bf9690..52752e0a5dc27 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -1021,6 +1021,7 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg, struct snd_soc_tplg_hdr *hdr) { struct snd_soc_dapm_context *dapm = &tplg->comp->dapm; + const size_t maxlen = SNDRV_CTL_ELEM_ID_NAME_MAXLEN; struct snd_soc_tplg_dapm_graph_elem *elem; struct snd_soc_dapm_route *route; int count, i; @@ -1044,38 +1045,27 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg, tplg->pos += sizeof(struct snd_soc_tplg_dapm_graph_elem);
/* validate routes */ - if (strnlen(elem->source, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == - SNDRV_CTL_ELEM_ID_NAME_MAXLEN) { - ret = -EINVAL; - break; - } - if (strnlen(elem->sink, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == - SNDRV_CTL_ELEM_ID_NAME_MAXLEN) { - ret = -EINVAL; - break; - } - if (strnlen(elem->control, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == - SNDRV_CTL_ELEM_ID_NAME_MAXLEN) { + if ((strnlen(elem->source, maxlen) == maxlen) || + (strnlen(elem->sink, maxlen) == maxlen) || + (strnlen(elem->control, maxlen) == maxlen)) { ret = -EINVAL; break; }
route->source = devm_kmemdup(tplg->dev, elem->source, - min(strlen(elem->source), - SNDRV_CTL_ELEM_ID_NAME_MAXLEN), + min(strlen(elem->source), maxlen), GFP_KERNEL); route->sink = devm_kmemdup(tplg->dev, elem->sink, - min(strlen(elem->sink), SNDRV_CTL_ELEM_ID_NAME_MAXLEN), + min(strlen(elem->sink), maxlen), GFP_KERNEL); if (!route->source || !route->sink) { ret = -ENOMEM; break; }
- if (strnlen(elem->control, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) != 0) { + if (strnlen(elem->control, maxlen) != 0) { route->control = devm_kmemdup(tplg->dev, elem->control, - min(strlen(elem->control), - SNDRV_CTL_ELEM_ID_NAME_MAXLEN), + min(strlen(elem->control), maxlen), GFP_KERNEL); if (!route->control) { ret = -ENOMEM;
[ 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.
Reported-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Closes: https://lore.kernel.org/linux-sound/171812236450.201359.3019210915105428447.... 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.... Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Amadeusz Sławiński amadeuszx.slawinski@linux.intel.com --- 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;
On Wed, Aug 14, 2024 at 05:47:47PM +0200, Amadeusz Sławiński wrote:
Commit 97ab304ecd95 ("ASoC: topology: Fix references to freed memory") is a problematic fix for issue in topology loading code, which was cherry-picked to stable. It was later corrected in 0298f51652be ("ASoC: topology: Fix route memory corruption"), however to apply cleanly e0e7bc2cbee9 ("ASoC: topology: Clean up route loading") also needs to be applied.
Link: https://lore.kernel.org/linux-sound/ZrwUCnrtKQ61LWFS@sashalap/T/#mbfd273adf8...
Should be applied to stable 6.1, 6.6, 6.9.
6.9 is long end-of-life (as shown on the front page of kernel.org.)
Patches queued up to 6.1 and 6.6 now, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Amadeusz Sławiński -
Greg Kroah-Hartman