ALSA: hda: cs35l41: Cleanup and fix double free in firmware request

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.

Fixes: cd40dad2ca91 ("ALSA: hda: cs35l41: Ensure firmware/tuning pairs are always loaded")
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>
Link: https://lore.kernel.org/r/20231003142138.180108-1-sbinding@opensource.cirrus.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index f9b7735..c6031f7 100644 (file)
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -185,10 +185,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 */
@@ -197,10 +201,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 */
@@ -215,10 +223,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 */
@@ -233,12 +245,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;
 }
 
@@ -254,7 +304,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 */
@@ -267,6 +316,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;
        }
 
@@ -286,32 +338,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)