Re: [PATCH v3 12/12] ALSA: hda/cs35l56: Add driver for Cirrus Logic CS35L56 amplifier

On 30/05/2023 08:39, Claudiu.Beznea@microchip.com wrote:

On 26.05.2023 15:28, Richard Fitzgerald wrote:

...

+static int cs35l56_hda_request_firmware_file(struct cs35l56_hda *cs35l56,

•                                        const struct firmware **firmware, char **filename,
  

•                                        const char *dir, const char *system_name,
  

•                                        const char *amp_name,
  

•                                        const char *filetype)
  

+{

•   char *s, c;
  

•   int ret = 0;
  

•   if (system_name && amp_name)
  

•           *filename = kasprintf(GFP_KERNEL, "%scs35l56%s-%02x-dsp1-misc-%s-%s.%s", dir,
  

•                                 cs35l56->base.secured ? "s" : "", cs35l56->base.rev,
  

•                                 system_name, amp_name, filetype);
  

•   else if (system_name)
  

•           *filename = kasprintf(GFP_KERNEL, "%scs35l56%s-%02x-dsp1-misc-%s.%s", dir,
  

•                                 cs35l56->base.secured ? "s" : "", cs35l56->base.rev,
  

•                                 system_name, filetype);
  

•   else
  

•           *filename = kasprintf(GFP_KERNEL, "%scs35l56%s-%02x-dsp1-misc.%s", dir,
  

•                                 cs35l56->base.secured ? "s" : "", cs35l56->base.rev,
  

•                                 filetype);
  

•   if (!*filename)
  

•           return -ENOMEM;
  

•   /*
  

•    * Make sure that filename is lower-case and any non alpha-numeric
  

•    * characters except full stop and forward slash are replaced with
  

•    * hyphens.
  

•    */
  

•   s = *filename;
  

•   while (*s) {
  

•           c = *s;
  

•           if (isalnum(c))
  

•                   *s = tolower(c);
  

•           else if (c != '.' && c != '/')
  

•                   *s = '-';
  

•           s++;
  

•   }
  

•   ret = firmware_request_nowarn(firmware, *filename, cs35l56->base.dev);
  

•   if (ret) {
  

•           dev_dbg(cs35l56->base.dev, "Failed to request '%s'\n", *filename);
  

•           kfree(*filename);
  

•           *filename = NULL;
  

•   } else {
  

•           dev_dbg(cs35l56->base.dev, "Found '%s'\n", *filename);
  

I may be wrong but *filename seems leaked to me on this path. I noticed it is could be duplicated on cs_dsp_debugfs_save_wmfwname() but the original copy seems left aside.

It's not a leak. The file has been found so the name of the found file is returned (it's used by other code). But there is a leak of that memory elsewhere.

...

•   }
  

•   return ret;
  

+}