[PATCH v1] ALSA: hda: cs35l41: Cleanup and fix double free in firmware request
From: Stefan Binding
Date: Tue Oct 03 2023 - 10:22:20 EST
There is an unlikely but possible double free when loading firmware,
and a missing free calls if a firmware is successfully requested but
the coefficient file request fails, leading to the fallback firmware
request occurring without clearing the previously loaded firmware.
Reported-by: kernel test robot <lkp@xxxxxxxxx>
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Closes: https://lore.kernel.org/r/202309291331.0JUUQnPT-lkp@xxxxxxxxx/
Signed-off-by: Stefan Binding <sbinding@xxxxxxxxxxxxxxxxxxxxx>
---
sound/pci/hda/cs35l41_hda.c | 115 +++++++++++++++++++++++++-----------
1 file changed, 79 insertions(+), 36 deletions(-)
diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index dd10b4cd3d1a..28cb10ddd191 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -188,10 +188,14 @@ static int cs35l41_request_firmware_files_spkid(struct cs35l41_hda *cs35l41,
cs35l41->speaker_id, "wmfw");
if (!ret) {
/* try cirrus/part-dspN-fwtype-sub<-spkidN><-ampname>.bin */
- return cs35l41_request_firmware_file(cs35l41, coeff_firmware, coeff_filename,
- CS35L41_FIRMWARE_ROOT,
- cs35l41->acpi_subsystem_id, cs35l41->amp_name,
- cs35l41->speaker_id, "bin");
+ ret = cs35l41_request_firmware_file(cs35l41, coeff_firmware, coeff_filename,
+ CS35L41_FIRMWARE_ROOT,
+ cs35l41->acpi_subsystem_id, cs35l41->amp_name,
+ cs35l41->speaker_id, "bin");
+ if (ret)
+ goto coeff_err;
+
+ return 0;
}
/* try cirrus/part-dspN-fwtype-sub<-ampname>.wmfw */
@@ -200,10 +204,14 @@ static int cs35l41_request_firmware_files_spkid(struct cs35l41_hda *cs35l41,
cs35l41->amp_name, -1, "wmfw");
if (!ret) {
/* try cirrus/part-dspN-fwtype-sub<-spkidN><-ampname>.bin */
- return cs35l41_request_firmware_file(cs35l41, coeff_firmware, coeff_filename,
- CS35L41_FIRMWARE_ROOT,
- cs35l41->acpi_subsystem_id, cs35l41->amp_name,
- cs35l41->speaker_id, "bin");
+ ret = cs35l41_request_firmware_file(cs35l41, coeff_firmware, coeff_filename,
+ CS35L41_FIRMWARE_ROOT,
+ cs35l41->acpi_subsystem_id, cs35l41->amp_name,
+ cs35l41->speaker_id, "bin");
+ if (ret)
+ goto coeff_err;
+
+ return 0;
}
/* try cirrus/part-dspN-fwtype-sub<-spkidN>.wmfw */
@@ -218,10 +226,14 @@ static int cs35l41_request_firmware_files_spkid(struct cs35l41_hda *cs35l41,
cs35l41->amp_name, cs35l41->speaker_id, "bin");
if (ret)
/* try cirrus/part-dspN-fwtype-sub<-spkidN>.bin */
- return cs35l41_request_firmware_file(cs35l41, coeff_firmware,
- coeff_filename, CS35L41_FIRMWARE_ROOT,
- cs35l41->acpi_subsystem_id, NULL,
- cs35l41->speaker_id, "bin");
+ ret = cs35l41_request_firmware_file(cs35l41, coeff_firmware,
+ coeff_filename, CS35L41_FIRMWARE_ROOT,
+ cs35l41->acpi_subsystem_id, NULL,
+ cs35l41->speaker_id, "bin");
+ if (ret)
+ goto coeff_err;
+
+ return 0;
}
/* try cirrus/part-dspN-fwtype-sub.wmfw */
@@ -236,12 +248,50 @@ static int cs35l41_request_firmware_files_spkid(struct cs35l41_hda *cs35l41,
cs35l41->speaker_id, "bin");
if (ret)
/* try cirrus/part-dspN-fwtype-sub<-spkidN>.bin */
- return cs35l41_request_firmware_file(cs35l41, coeff_firmware,
- coeff_filename, CS35L41_FIRMWARE_ROOT,
- cs35l41->acpi_subsystem_id, NULL,
- cs35l41->speaker_id, "bin");
+ ret = cs35l41_request_firmware_file(cs35l41, coeff_firmware,
+ coeff_filename, CS35L41_FIRMWARE_ROOT,
+ cs35l41->acpi_subsystem_id, NULL,
+ cs35l41->speaker_id, "bin");
+ if (ret)
+ goto coeff_err;
+ }
+
+ return ret;
+coeff_err:
+ release_firmware(*wmfw_firmware);
+ kfree(*wmfw_filename);
+ return ret;
+}
+
+static int cs35l41_fallback_firmware_file(struct cs35l41_hda *cs35l41,
+ const struct firmware **wmfw_firmware,
+ char **wmfw_filename,
+ const struct firmware **coeff_firmware,
+ char **coeff_filename)
+{
+ int ret;
+
+ /* Handle fallback */
+ dev_warn(cs35l41->dev, "Falling back to default firmware.\n");
+
+ /* fallback try cirrus/part-dspN-fwtype.wmfw */
+ ret = cs35l41_request_firmware_file(cs35l41, wmfw_firmware, wmfw_filename,
+ CS35L41_FIRMWARE_ROOT, NULL, NULL, -1, "wmfw");
+ if (ret)
+ goto err;
+
+ /* fallback try cirrus/part-dspN-fwtype.bin */
+ ret = cs35l41_request_firmware_file(cs35l41, coeff_firmware, coeff_filename,
+ CS35L41_FIRMWARE_ROOT, NULL, NULL, -1, "bin");
+ if (ret) {
+ release_firmware(*wmfw_firmware);
+ kfree(*wmfw_filename);
+ goto err;
}
+ return 0;
+err:
+ dev_warn(cs35l41->dev, "Unable to find firmware and tuning\n");
return ret;
}
@@ -257,7 +307,6 @@ static int cs35l41_request_firmware_files(struct cs35l41_hda *cs35l41,
ret = cs35l41_request_firmware_files_spkid(cs35l41, wmfw_firmware, wmfw_filename,
coeff_firmware, coeff_filename);
goto out;
-
}
/* try cirrus/part-dspN-fwtype-sub<-ampname>.wmfw */
@@ -270,6 +319,9 @@ static int cs35l41_request_firmware_files(struct cs35l41_hda *cs35l41,
CS35L41_FIRMWARE_ROOT,
cs35l41->acpi_subsystem_id, cs35l41->amp_name,
-1, "bin");
+ if (ret)
+ goto coeff_err;
+
goto out;
}
@@ -289,32 +341,23 @@ static int cs35l41_request_firmware_files(struct cs35l41_hda *cs35l41,
CS35L41_FIRMWARE_ROOT,
cs35l41->acpi_subsystem_id, NULL, -1,
"bin");
+ if (ret)
+ goto coeff_err;
}
out:
- if (!ret)
- return 0;
+ if (ret)
+ /* if all attempts at finding firmware fail, try fallback */
+ goto fallback;
- /* Handle fallback */
- dev_warn(cs35l41->dev, "Falling back to default firmware.\n");
+ return 0;
+coeff_err:
release_firmware(*wmfw_firmware);
kfree(*wmfw_filename);
-
- /* fallback try cirrus/part-dspN-fwtype.wmfw */
- ret = cs35l41_request_firmware_file(cs35l41, wmfw_firmware, wmfw_filename,
- CS35L41_FIRMWARE_ROOT, NULL, NULL, -1, "wmfw");
- if (!ret)
- /* fallback try cirrus/part-dspN-fwtype.bin */
- ret = cs35l41_request_firmware_file(cs35l41, coeff_firmware, coeff_filename,
- CS35L41_FIRMWARE_ROOT, NULL, NULL, -1, "bin");
-
- if (ret) {
- release_firmware(*wmfw_firmware);
- kfree(*wmfw_filename);
- dev_warn(cs35l41->dev, "Unable to find firmware and tuning\n");
- }
- return ret;
+fallback:
+ return cs35l41_fallback_firmware_file(cs35l41, wmfw_firmware, wmfw_filename,
+ coeff_firmware, coeff_filename);
}
#if IS_ENABLED(CONFIG_EFI)
--
2.34.1