linux-kernel - [PATCH] ASoC: topology: fix stack corruption by code unification for creating standalone and widget controls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <7eca678fa7faa9e160b998192e87220de81315c8.1726847965.git.hns@goldelico.com>
Date: Fri, 20 Sep 2024 17:59:33 +0200
From: "H. Nikolaus Schaller" <hns@...delico.com>
To: Amadeusz Sławiński <amadeuszx.slawinski@...ux.intel.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>,
	Jaroslav Kysela <perex@...ex.cz>,
	Takashi Iwai <tiwai@...e.com>
Cc: linux-sound@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	letux-kernel@...nphoenux.org,
	kernel@...a-handheld.com,
	risca@...akolonin.se,
	"H. Nikolaus Schaller" <hns@...delico.com>
Subject: [PATCH] ASoC: topology: fix stack corruption by code unification for creating standalone and widget controls

After finding kernel crashes on omap4/5 aess on PandaES and OMAP5UEVM like

[   46.367041] Unable to handle kernel execution of memory at virtual address f164d95c when execute

a bisect did initially hint at commit 8f2942b9198c9 ("ASoC: topology: Unify
code for creating standalone and widget enum control")

Deeper analysis shows a bug in two of the three "ASoC: topology: Unify code"
patches. There, a variable is initialized to point into the struct snd_kcontrol_new
on stack instead of the newly devm_kzalloc'ed memory.

Hence, initialization through struct soc_mixer_control or struct soc_bytes_ext
members overwrites stack instead of the intended target address, leading to
the observed kernel crashes.

Fixes: 8f2942b9198c ("ASoC: topology: Unify code for creating standalone and widget enum control")
Fixes: 4654ca7cc8d6 ("ASoC: topology: Unify code for creating standalone and widget mixer control").
Fixes: 0867278200f7 ("ASoC: topology: Unify code for creating standalone and widget bytes control").
Tested-by: risca@...akolonin.se
Signed-off-by: H. Nikolaus Schaller <hns@...delico.com>
---

Notes:
    There seems to be another weakness of all three patches. The initialization
    of the kc.private_value is now done after calling soc_tplg_control_load()
    which may pass the incompletely initialized control down to some control_load()
    operation (if tplg->ops are defined).
    
    Since this feature is not used by the omap4/5 aess subsystem drivers it is
    not taken care of by this fix.
    
    Another general observation with this code (not related to these patches here)
    is that it does not appear to be 64 bit address safe since private_value of
    struct snd_kcontrol_new is 'unsigned long' [1] but used to store a pointer.
    
    This is fine on omap4/5 since they are 32 bit machines with 32 bit address
    range. A problem would be a machine with 32 bit unsigned long and >32 bit
    addresses.
    
    [1] According to my research this definition was done before Linux-2.6.12-rc2

 sound/soc/soc-topology.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index af5d42b57be7e..3d82570293b29 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -889,7 +889,7 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, size_t size)
 		return ret;
 
 	/* register dynamic object */
-	sbe = (struct soc_bytes_ext *)&kc.private_value;
+	sbe = (struct soc_bytes_ext *)kc.private_value;
 
 	INIT_LIST_HEAD(&sbe->dobj.list);
 	sbe->dobj.type = SND_SOC_DOBJ_BYTES;
@@ -923,7 +923,7 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, size_t size)
 		return ret;
 
 	/* register dynamic object */
-	sm = (struct soc_mixer_control *)&kc.private_value;
+	sm = (struct soc_mixer_control *)kc.private_value;
 
 	INIT_LIST_HEAD(&sm->dobj.list);
 	sm->dobj.type = SND_SOC_DOBJ_MIXER;
-- 
2.44.0