From: Stefan Binding <sbinding@opensource.cirrus.com>
To: Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>
Cc: <alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>,
<patches@opensource.cirrus.com>,
Stefan Binding <sbinding@opensource.cirrus.com>,
kernel test robot <lkp@intel.com>,
"Dan Carpenter" <dan.carpenter@linaro.org>
Subject: [PATCH v1] ALSA: hda: cs35l41: Cleanup and fix double free in firmware request
Date: Tue, 3 Oct 2023 15:21:38 +0100 [thread overview]
Message-ID: <20231003142138.180108-1-sbinding@opensource.cirrus.com> (raw)
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@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202309291331.0JUUQnPT-lkp@intel.com/
Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
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
next reply other threads:[~2023-10-03 14:23 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-03 14:21 Stefan Binding [this message]
2023-10-06 9:06 ` [PATCH v1] ALSA: hda: cs35l41: Cleanup and fix double free in firmware request Takashi Iwai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231003142138.180108-1-sbinding@opensource.cirrus.com \
--to=sbinding@opensource.cirrus.com \
--cc=alsa-devel@alsa-project.org \
--cc=dan.carpenter@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp@intel.com \
--cc=patches@opensource.cirrus.com \
--cc=perex@perex.cz \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).