ASoC: tas2781: check the validity of prm_no/cfg_no

Commit Message

Gergo Koteles Dec. 14, 2023, 10:04 p.m. UTC

Add additional checks for program/config numbers to avoid loading from
invalid addresses.

If prm_no/cfg_no is negative, skip uploading program/config.

The tas2781-hda driver caused a NULL pointer dereference after loading
module, and before first runtime_suspend.

the state was:
tas_priv->cur_conf = -1;
tas_priv->tasdevice[i].cur_conf = 0;
program = &(tas_fmw->programs[-1]);

BUG: kernel NULL pointer dereference, address: 0000000000000010
Call Trace:
 <TASK>
 ? __die+0x23/0x70
 ? page_fault_oops+0x171/0x4e0
 ? vprintk_emit+0x175/0x2b0
 ? exc_page_fault+0x7f/0x180
 ? asm_exc_page_fault+0x26/0x30
 ? tasdevice_load_block_kernel+0x21/0x310 [snd_soc_tas2781_fmwlib]
 tasdevice_select_tuningprm_cfg+0x268/0x3a0 [snd_soc_tas2781_fmwlib]
 tasdevice_tuning_switch+0x69/0x710 [snd_soc_tas2781_fmwlib]
 tas2781_hda_playback_hook+0xd4/0x110 [snd_hda_scodec_tas2781_i2c]

Fixes: 915f5eadebd2 ("ASoC: tas2781: firmware lib")
CC: [e-mail]
Signed-off-by: Gergo Koteles <[e-mail]>
---
 sound/soc/codecs/tas2781-fmwlib.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)


base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa

Comments

Mark Brown Dec. 15, 2023, 5:11 p.m. UTC | #1

On Thu, 14 Dec 2023 23:04:44 +0100, Gergo Koteles wrote:
> Add additional checks for program/config numbers to avoid loading from
> invalid addresses.
> 
> If prm_no/cfg_no is negative, skip uploading program/config.
> 
> The tas2781-hda driver caused a NULL pointer dereference after loading
> module, and before first runtime_suspend.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: tas2781: check the validity of prm_no/cfg_no
      commit: f32c80d34249e1cfb2e647ab3c8ef38a460c787f

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Patch

diff --git a/sound/soc/codecs/tas2781-fmwlib.c b/sound/soc/codecs/tas2781-fmwlib.c
index eb55abae0d7b..1dfac9b2fca2 100644
--- a/sound/soc/codecs/tas2781-fmwlib.c
+++ b/sound/soc/codecs/tas2781-fmwlib.c
@@ -2219,11 +2219,11 @@  int tasdevice_select_tuningprm_cfg(void *context, int prm_no,
 		goto out;
 	}
 
-	conf = &(tas_fmw->configs[cfg_no]);
 	for (i = 0, prog_status = 0; i < tas_priv->ndev; i++) {
 		if (cfg_info[rca_conf_no]->active_dev & (1 << i)) {
-			if (tas_priv->tasdevice[i].cur_prog != prm_no
-				|| tas_priv->force_fwload_status) {
+			if (prm_no >= 0
+				&& (tas_priv->tasdevice[i].cur_prog != prm_no
+				|| tas_priv->force_fwload_status)) {
 				tas_priv->tasdevice[i].cur_conf = -1;
 				tas_priv->tasdevice[i].is_loading = true;
 				prog_status++;
@@ -2258,7 +2258,8 @@  int tasdevice_select_tuningprm_cfg(void *context, int prm_no,
 	}
 
 	for (i = 0, status = 0; i < tas_priv->ndev; i++) {
-		if (tas_priv->tasdevice[i].cur_conf != cfg_no
+		if (cfg_no >= 0
+			&& tas_priv->tasdevice[i].cur_conf != cfg_no
 			&& (cfg_info[rca_conf_no]->active_dev & (1 << i))
 			&& (tas_priv->tasdevice[i].is_loaderr == false)) {
 			status++;
@@ -2268,6 +2269,7 @@  int tasdevice_select_tuningprm_cfg(void *context, int prm_no,
 	}
 
 	if (status) {
+		conf = &(tas_fmw->configs[cfg_no]);
 		status = 0;
 		tasdevice_load_data(tas_priv, &(conf->dev_data));
 		for (i = 0; i < tas_priv->ndev; i++) {
@@ -2311,7 +2313,7 @@  int tasdevice_prmg_load(void *context, int prm_no)
 	}
 
 	for (i = 0, prog_status = 0; i < tas_priv->ndev; i++) {
-		if (tas_priv->tasdevice[i].cur_prog != prm_no) {
+		if (prm_no >= 0 && tas_priv->tasdevice[i].cur_prog != prm_no) {
 			tas_priv->tasdevice[i].cur_conf = -1;
 			tas_priv->tasdevice[i].is_loading = true;
 			prog_status++;
@@ -2356,7 +2358,7 @@  int tasdevice_prmg_calibdata_load(void *context, int prm_no)
 	}
 
 	for (i = 0, prog_status = 0; i < tas_priv->ndev; i++) {
-		if (tas_priv->tasdevice[i].cur_prog != prm_no) {
+		if (prm_no >= 0 && tas_priv->tasdevice[i].cur_prog != prm_no) {
 			tas_priv->tasdevice[i].cur_conf = -1;
 			tas_priv->tasdevice[i].is_loading = true;
 			prog_status++;